January 2022 - Wine-Devel - list.winehq.org

Re: [RFC PATCH v2 04/11] server: Attempt to complete I/O request immediately in recv_socket.

by Jinoh Kang

On 1/26/22 03:23, Zebediah Figura (she/her) wrote: > On 1/22/22 08:36, Jinoh Kang wrote: >> diff --git a/include/wine/afd.h b/include/wine/afd.h >> index efd5787e90a..1e26739229d 100644 >> --- a/include/wine/afd.h >> +++ b/include/wine/afd.h >> @@ -37,6 +37,23 @@ struct afd_wsabuf_32 >> # define WS(x) x >> #endif >> + >> +/* Used in the iosb.result field to indicate that the current socket I/O >> + * operation is in synchronous non-blocking mode. This value is normally >> + * transmitted via the APC_ASYNC_IO system APC call (with status STATUS_ALERTED) >> + * when the server gives the client a chance to complete the I/O synchronously >> + * before resuming the request as fully asynchronous I/O or failing it. >> + * If the I/O fails with EWOULDBLOCK and the iosb.result field is set to any >> + * other value, the client shall request the server to resume the asynchronous >> + * operation. >> + * >> + * The value (ULONG_PTR)-1 (the maximum value of ULONG_PTR) is chosen so that >> + * it will be least likely to be confused with "the number of bytes transferred >> + * so far." Any I/O operation that has made it to the maximum number of bytes >> + * shall complete immediately anyway. >> + */ >> +#define AFD_WINE_IN_NONBLOCKING_MODE ((ULONG_PTR)-1) >> + > > Why add this instead of returning a separate field from the recv_socket request? It's not intended to be returned from recv_socket request. > This value is normally transmitted via the APC_ASYNC_IO system APC call (with status STATUS_ALERTED) > transmitted via the APC_ASYNC_IO system APC call (with status STATUS_ALERTED) > when the server gives the client a chance to complete the I/O synchronously Do you have any suggestion as to how to better pass the blocking/nonblocking flag to async_recv_proc()? -- Sincerely, Jinoh Kang

Jan. 26, 2022

Re: [RFC PATCH v2 03/11] include: Add a helper function to test if a status indicates deferred completion.

by Jinoh Kang

On 1/26/22 03:21, Zebediah Figura (she/her) wrote: > On 1/22/22 08:36, Jinoh Kang wrote: >> Put it in a separate header file so that it won't pollute everyone's >> pre-processor macro definition namespace with ntstatus.h. Note that >> winnt.h omits STATUS_ALERTED. > > NTSTATUS values are already defined by all server files; Sorry for the confusion (I'll squash up some of the patches as you've suggested anyway). The header is used by both the server and the client; hence its placement in include/. > there's no need for a separate header. If a translation unit includes winnt.h but _not_ ntstatus.h, the compiler complains that STATUS_ALERTED is not defined. Perhaps we can just move STATUS_ALERTED into winnt.h instead? > The NTSTATUS typedef isn't, but we use "unsigned int" anyway. Server code can already use NTSTATUS, so there's no problem with it. Nevertheless, there's nothing particularly wrong here with using "unsigned int" too. -- Sincerely, Jinoh Kang

Jan. 26, 2022

Re: [PATCH vkd3d] build: Disable crosstests as well if tests are disabled.

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=106014 Your paranoid android. === debian11 (build log) === error: patch failed: configure.ac:148 Task: Patch failed to apply === debian11 (build log) === error: patch failed: configure.ac:148 Task: Patch failed to apply

Jan. 26, 2022

Re: [PATCH 4/5] winhttp: Return error from WinHttpWebSocketQueryCloseStatus() if close frame was not received.

by Marvin

Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=106012 Your paranoid android. === w8adm (32 bit report) === winhttp: notification.c:256: Test failed: failed to send request 12007 notification.c:258: Test failed: unexpected function 3, expected 4. probably some notifications were missing notification.c:260: Test failed: failed to receive response 12019 notification.c:264: Test failed: failed unexpectedly 12019 notification.c:265: Test failed: request failed unexpectedly 0 notification.c:268: Test failed: unexpected function 3, expected 13. probably some notifications were missing notification.c:116: Test failed: 268: expected status 0x00000002 got 0x00000800 notification.c:117: Test failed: 268: expected function 3 got 13 notification: Timeout

Jan. 26, 2022

Re: [PATCH 5/8] secur32: Add support for setting DTLS timeouts.

by Connor McAdams

On Tue, Jan 25, 2022 at 10:32:11AM -0500, Connor McAdams wrote: > Add support for setting the DTLS timeout values, and set the > retransmission timeout value to 0 to allow for retransmission on each > call to schan_InitializeSecurityContext. > > Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> > --- > dlls/secur32/schannel.c | 6 ++++++ > dlls/secur32/schannel_gnutls.c | 22 ++++++++++++++++++++++ > dlls/secur32/secur32_priv.h | 8 ++++++++ > 3 files changed, 36 insertions(+) > > diff --git a/dlls/secur32/schannel.c b/dlls/secur32/schannel.c > index fa5577d78e3..19e21da8d72 100644 > --- a/dlls/secur32/schannel.c > +++ b/dlls/secur32/schannel.c > @@ -809,6 +809,12 @@ static SECURITY_STATUS SEC_ENTRY schan_InitializeSecurityContextW( > else WARN("invalid buffer size %u\n", buffer->cbBuffer); > } > > + if (is_dtls_context(ctx)) > + { > + struct set_dtls_timeouts_params params = { ctx->transport.session, 0, 60000 }; > + GNUTLS_CALL( set_dtls_timeouts, &params ); > + } > + > phNewContext->dwLower = handle; > phNewContext->dwUpper = 0; > } > diff --git a/dlls/secur32/schannel_gnutls.c b/dlls/secur32/schannel_gnutls.c > index 31fdb769677..ac51cbb5d9f 100644 > --- a/dlls/secur32/schannel_gnutls.c > +++ b/dlls/secur32/schannel_gnutls.c > @@ -60,6 +60,7 @@ static int (*pgnutls_cipher_get_block_size)(gnutls_cipher_algorithm_t); > static void (*pgnutls_transport_set_pull_timeout_function)(gnutls_session_t, > int (*)(gnutls_transport_ptr_t, unsigned int)); > static void (*pgnutls_dtls_set_mtu)(gnutls_session_t, unsigned int); > +static void (*pgnutls_dtls_set_timeouts)(gnutls_session_t, unsigned int, unsigned int); > > /* Not present in gnutls version < 3.2.0. */ > static int (*pgnutls_alpn_get_selected_protocol)(gnutls_session_t, gnutls_datum_t *); > @@ -198,6 +199,12 @@ static void compat_gnutls_dtls_set_mtu(gnutls_session_t session, unsigned int mt > FIXME("\n"); > } > > +static void compat_gnutls_dtls_set_timeouts(gnutls_session_t session, unsigned int retrans_timeout, > + unsigned int total_timeout) > +{ > + FIXME("\n"); > +} > + > static void init_schan_buffers(struct schan_buffers *s, const PSecBufferDesc desc, > int (*get_next_buffer)(const struct schan_transport *, struct schan_buffers *)) > { > @@ -989,6 +996,15 @@ static NTSTATUS schan_set_dtls_mtu( void *args ) > return SEC_E_OK; > } > > +static NTSTATUS schan_set_dtls_timeouts( void *args ) > +{ > + const struct set_dtls_timeouts_params *params = args; > + gnutls_session_t s = (gnutls_session_t)params->session; > + > + pgnutls_dtls_set_timeouts(s, params->retrans_timeout, params->total_timeout); > + return SEC_E_OK; > +} > + > static inline void reverse_bytes(BYTE *buf, ULONG len) > { > BYTE tmp; > @@ -1245,6 +1261,11 @@ static NTSTATUS process_attach( void *args ) > WARN("gnutls_dtls_set_mtu not found\n"); > pgnutls_dtls_set_mtu = compat_gnutls_dtls_set_mtu; > } > + if (!(pgnutls_dtls_set_timeouts = dlsym(libgnutls_handle, "gnutls_dtls_set_timeouts"))) > + { > + WARN("gnutls_dtls_set_timeouts not found\n"); > + pgnutls_dtls_set_timeouts = compat_gnutls_dtls_set_timeouts; > + } > if (!(pgnutls_privkey_export_x509 = dlsym(libgnutls_handle, "gnutls_privkey_export_x509"))) > { > WARN("gnutls_privkey_export_x509 not found\n"); > @@ -1308,6 +1329,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = > schan_set_application_protocols, > schan_set_dtls_mtu, > schan_set_session_target, > + schan_set_dtls_timeouts, > }; > > #endif /* SONAME_LIBGNUTLS */ > diff --git a/dlls/secur32/secur32_priv.h b/dlls/secur32/secur32_priv.h > index c17adc96dc6..64edc0581d6 100644 > --- a/dlls/secur32/secur32_priv.h > +++ b/dlls/secur32/secur32_priv.h > @@ -203,6 +203,13 @@ struct set_session_target_params > const char *target; > }; > > +struct set_dtls_timeouts_params > +{ > + schan_session session; > + unsigned int retrans_timeout; > + unsigned int total_timeout; > +}; > + > enum schan_funcs > { > unix_process_attach, > @@ -225,6 +232,7 @@ enum schan_funcs > unix_set_application_protocols, > unix_set_dtls_mtu, > unix_set_session_target, > + unix_set_dtls_timeouts, > }; > > #endif /* __SECUR32_PRIV_H__ */ > -- > 2.25.1 > All patches prior to this one are good, but this one and beyond will break things. Maybe this is the universe's way of telling me to send smaller patch sets. ;) Essentially, the issue seems to be that we need to go back to non-blocking mode with gnutls on DTLS contexts. We behave in a non-blocking manner to begin with, seeing as we return immediately from the pull_timeout callback. If we don't do this, it takes all sorts of effort to work around it, and make it not get into a weird state when no data is received. If it isn't in non-blocking mode, it doesn't seem to complete a handshake if retransmission is involved. Is there any good reason we switched away from GNUTLS_NONBLOCK in the first place? If so, I can try to work around it. But from my testing, having it enabled doesn't seem to cause issues. But maybe I'm missing something.

Jan. 26, 2022

[PATCH vkd3d] build: Disable crosstests as well if tests are disabled.

by Zebediah Figura

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- configure.ac | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index d296dfd4b..f69f1ea4a 100644 --- a/configure.ac +++ b/configure.ac @@ -148,7 +148,7 @@ AC_CONFIG_FILES([Makefile]) AC_OUTPUT dnl Output configuration summary -AS_IF([test "x$CROSSTARGET32" != "xno" -o "x$CROSSTARGET64" != "xno"], +AS_IF([test "x$enable_tests" != "xno" -a $ "x$CROSSTARGET32" != "xno" -o "x$CROSSTARGET64" != "xno" $], [HAVE_CROSSTEST=yes], [HAVE_CROSSTEST=no]) AS_IF([test $DX_FLAG_doc = 1], [HAVE_DOCS=yes], [HAVE_DOCS=no]) @@ -166,6 +166,6 @@ AS_ECHO([" Building documentation: ${HAVE_DOCS} Building crosstests: ${HAVE_CROSSTEST}"]) -AS_IF([test "x$CROSSTARGET32" != "xno"], [AS_ECHO([" Using 32-bit cross compiler: $CROSSCC32"])]) -AS_IF([test "x$CROSSTARGET64" != "xno"], [AS_ECHO([" Using 64-bit cross compiler: $CROSSCC64"])]) +AS_IF([test "$HAVE_CROSSTEST" == "yes" -a "x$CROSSTARGET32" != "xno"], [AS_ECHO([" Using 32-bit cross compiler: $CROSSCC32"])]) +AS_IF([test "$HAVE_CROSSTEST" == "yes" -a "x$CROSSTARGET64" != "xno"], [AS_ECHO([" Using 64-bit cross compiler: $CROSSCC64"])]) AS_ECHO([]) -- 2.34.1

Jan. 26, 2022

Re: [PATCH] Increased winsock2 maximum sockets

by Louis Lenders

>I wasn't claiming there's no reason behind it, just asking if there are >any, for documentation's sake i see many patches going in without documentation (or patches falling of the list without documentation) so I can imagine why he sent in without documentation... . o Op wo 26 jan. 2022 om 00:44 schreef Zebediah Figura (she/her) < zfigura(a)codeweavers.com>: > On 1/25/22 17:22, Benjamin Shefte wrote: > > this mostdefinitaly has affected multiple server applications I use for > the > > old WON.Net Backend servers, so you cant say this is a patch without any > > reason befind it > > I wasn't claiming there's no reason behind it, just asking if there are > any, for documentation's sake. > > The more salient problem with the patch is the lack of tests. It should > be easy to prove this change's correctness. > >

Jan. 26, 2022

Re: [PATCH] WBEMProxy: Implemented Complete SoftwareLicensingProduct row and data

by Fabian Maurer

Hello Benjamin, what's up with this attitude? Don't take it personally, just try to plead your case. Surely you have a reason to want those changes to be included? Regards, Fabian Maurer On Mittwoch, 26. Januar 2022 00:37:23 CET Benjamin Shefte wrote: > Does that mean just because no person has made an bugreport about it yet, > it isn't needed now? and then in the future people with the issue will have > to wait however many months for a full patch? > > seems a bit asinine and silly... Im thinking its time to fork wine into a > less strict development enviroment where developers can code more freely > not only deal with bugs releasted to games.. which linux shoulndt be used > for anyway. >

Jan. 26, 2022

[PATCH 5/5] winhttp: Don't send pong on timeout in socket_receive().

by Paul Gofman

Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- As testing involves waiting for timeouts it doesn't look good for inclusion into the test suite. I've tested this change separately: https://gist.github.com/gofman/bd2f3b6fd6cbe993e97353560ce81e04 This test succeeds on Windows for me but fails without this patch in Wine: the server close status received during closing handshake becomes WINHTTP_WEB_SOCKET_PROTOCOL_ERROR_CLOSE_STATUS (1002). Sending ping instead of pong causes the server to send a pong reply as expected although I still see status 1002 on close after. Which probably suggests that Windows doesn't send the ping as well (I also tried that with WINHTTP_OPTION_WEB_SOCKET_KEEPALIVE_INTERVAL socket option set to 15000 and Sleep to 30000). Also, while looking at this I spotted that we currently ignore PING application data (which may be present according to rfc6455 and should be echoed back with PONG. But I didn't see any server sending pings yet and didn't do anything with this part so far. dlls/winhttp/request.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 62de9f5cff9..385a4fc4e55 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -3600,7 +3600,11 @@ static DWORD socket_receive( struct socket *socket, void *buf, DWORD len, DWORD { if (!(socket->opcode & CONTROL_BIT) || (ret = handle_control_frame( socket ))) break; } - else if (ret == WSAETIMEDOUT) ret = socket_send_pong( socket ); + else if (ret == WSAETIMEDOUT) + { + WARN( "timeout, retrying.\n" ); + continue; + } if (ret) break; } } -- 2.34.1

Jan. 25, 2022

[PATCH 4/5] winhttp: Return error from WinHttpWebSocketQueryCloseStatus() if close frame was not received.

by Paul Gofman

Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 6 ++++++ dlls/winhttp/tests/notification.c | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 35a62eb4e2b..62de9f5cff9 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -3906,6 +3906,12 @@ DWORD WINAPI WinHttpWebSocketQueryCloseStatus( HINTERNET hsocket, USHORT *status return ERROR_INVALID_OPERATION; } + if (!socket->close_frame_received || socket->close_frame_receive_err) + { + ret = socket->close_frame_received ? socket->close_frame_receive_err : ERROR_INVALID_OPERATION; + release_object( &socket->hdr ); + return ret; + } *status = socket->status; *ret_len = socket->reason_len; if (socket->reason_len > len) ret = ERROR_INSUFFICIENT_BUFFER; diff --git a/dlls/winhttp/tests/notification.c b/dlls/winhttp/tests/notification.c index 2db5a094f87..1abc8cf377f 100644 --- a/dlls/winhttp/tests/notification.c +++ b/dlls/winhttp/tests/notification.c @@ -971,9 +971,9 @@ static void test_websocket(BOOL secure) close_status = 0xdead; size = sizeof(buffer) + 1; err = pWinHttpWebSocketQueryCloseStatus( socket, &close_status, buffer, sizeof(buffer), &size ); - todo_wine ok( err == ERROR_INVALID_OPERATION, "got %u\n", err ); - todo_wine ok( close_status == 0xdead, "got %u\n", close_status ); - todo_wine ok( size == sizeof(buffer) + 1, "got %u\n", size ); + ok( err == ERROR_INVALID_OPERATION, "got %u\n", err ); + ok( close_status == 0xdead, "got %u\n", close_status ); + ok( size == sizeof(buffer) + 1, "got %u\n", size ); WaitForSingleObject( info.wait, INFINITE ); -- 2.34.1

Jan. 25, 2022