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

[RFC PATCH 0/5] Fix sock_recv reordering issue (#52401): unknown_status + wineserver call approach
by Jinoh Kang Jan. 27, 2022

Jan. 27, 2022

<preamble> This is my forth attempt to tackle the issue #52401 [1]: "Improper synchronization in sock_recv/sock_send leads to arbitrary reordering of completion of I/O requests", which was initially reported (outside Bugzilla) by Dongwan Kim [2]. Basically, the old process of sock_recv is: 1. Perform the I/O synchronously. 2. Report the result back to the server, or queue up the request in the server. 3. If blocking, wait for I/O to complete. The new process of sock_recv would be: 1. Queue up the request in the server. 2. Perform the I/O synchronously. 3. Report the result back to the server, 4. If blocking, wait for I/O to complete. Everything except the actual I/O requires communicating with wineserver. My goal here is to fix the issue without introducing extra calls to wineserver. (However, if it turns out that this goal is not of utmost significance, then this patch serie can be easily modified so that it issues separate server calls.) The previous approaches are listed here: - Add a new select type called SELECT_SIGNAL_WAIT_ASYNC [3]. Zebediah has pointed out [4] that it is not very elegant to (ab-)use the synchronization machinery for communicating the async result. - Use APC_ASYNC_IO to perform the synchronous I/O [5]. This ended up with a total of 11 patches, and turned out to be rather too complicated for a simple task. - Add a new wineserver call, and use "add_queue" hook to save a round trip to the server [6]. </preamble> Each of the above turned out to be either too intrusive, complicated, or hard to verify. Per suggestion by Zebediah [7], this is my (hopefully) last approach: keep the new wineserver call, but keep out the implicit STATUS_PENDING transition from "add_queue"; also, (re-)use the "unknown_status" field. [1] https://bugs.winehq.org/show_bug.cgi?id=52401 [2] https://www.winehq.org/pipermail/wine-devel/2021-May/186454.html [3] https://www.winehq.org/pipermail/wine-devel/2022-January/204695.html [4] https://www.winehq.org/pipermail/wine-devel/2022-January/204710.html [5] https://www.winehq.org/pipermail/wine-devel/2022-January/205168.html [6] https://www.winehq.org/pipermail/wine-devel/2022-January/205193.html [7] https://www.winehq.org/pipermail/wine-devel/2022-January/205738.html Jinoh Kang (5): server: Allow calling async_handoff() with status code STATUS_ALERTED. server: Add a new server request "notify_async_direct_result." server: Attempt to complete I/O request immediately in recv_socket. ntdll: Don't call try_recv before server call in sock_recv. server: Replace redundant recv_socket status fields with force_async boolean field. dlls/ntdll/unix/socket.c | 37 +++++++++------- dlls/ntdll/unix/sync.c | 22 +++++++++ dlls/ntdll/unix/unix_private.h | 1 + dlls/ws2_32/tests/sock.c | 8 ++-- server/async.c | 81 ++++++++++++++++++++++++++++++++++ server/protocol.def | 14 +++++- server/sock.c | 40 ++++++++++++----- 7 files changed, 168 insertions(+), 35 deletions(-) -- 2.34.1

1 5

[PATCH] secur32: Fix trace in schannel pull_adapter function.
by Connor McAdams Jan. 27, 2022

Jan. 27, 2022

Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/secur32/schannel_gnutls.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/secur32/schannel_gnutls.c b/dlls/secur32/schannel_gnutls.c index 31fdb769677..26f9a6aff23 100644 --- a/dlls/secur32/schannel_gnutls.c +++ b/dlls/secur32/schannel_gnutls.c @@ -365,7 +365,7 @@ static ssize_t pull_adapter(gnutls_transport_ptr_t transport, void *buff, size_t SIZE_T len = buff_len; char *b; - TRACE("Push %lu bytes\n", len); + TRACE("Pull %lu bytes\n", len); b = get_buffer(t, &t->in, &len); if (!b) @@ -375,7 +375,7 @@ static ssize_t pull_adapter(gnutls_transport_ptr_t transport, void *buff, size_t } memcpy(buff, b, len); t->in.offset += len; - TRACE("Wrote %lu bytes\n", len); + TRACE("Read %lu bytes\n", len); return len; } -- 2.25.1

2 1

Re: [PATCH] Increased winsock2 maximum sockets
by Benjamin Shefte Jan. 27, 2022

Jan. 27, 2022

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 On Tue, Jan 25, 2022 at 3:42 PM Marvin <testbot(a)winehq.org> wrote: > Thank you for your contribution to Wine! > > This is an automated notification to let you know that your patch has > been reviewed and its status set to "Rejected". > > This means that the patch has been rejected by a reviewer. You should > have received a mail explaining why it was rejected. You need to fix > the issue and resend the patch, or if you are convinced that your > patch is good as is, you should reply to the rejection message with > your counterarguments. > > If you do not understand the reason for this status, disagree with our > assessment, or are simply not sure how to proceed next, please ask for > clarification by replying to this email. >

3 3

[PATCH 1/2] winmm/tests: Demonstrate that WOM_DONE is not invoked reentrantly
by Andrew Eikum Jan. 27, 2022

Jan. 27, 2022

From: Liam Murphy <liampm32(a)gmail.com> Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> --- dlls/winmm/tests/wave.c | 103 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/dlls/winmm/tests/wave.c b/dlls/winmm/tests/wave.c index 2f0c8443363..56501367190 100644 --- a/dlls/winmm/tests/wave.c +++ b/dlls/winmm/tests/wave.c @@ -1626,6 +1626,108 @@ static void test_fragmentsize(void) CloseHandle(hevent); } +static void CALLBACK test_reentrant_callback_func(HWAVEOUT hwo, UINT uMsg, + DWORD_PTR dwInstance, + DWORD_PTR dwParam1, DWORD_PTR dwParam2) +{ + static int wom_done_count = 0; + + switch(uMsg){ + case WOM_OPEN: + case WOM_CLOSE: + ok(GetCurrentThreadId() == g_tid, "Got different thread ID\n"); + break; + + case WOM_DONE: + /* verify that WOM_DONE is not sent during the following waveOutWrite */ + todo_wine_if(wom_done_count == 1) + ok(g_tid == 0, "callback called reentrantly\n"); + + g_tid = GetCurrentThreadId(); + + if(wom_done_count++ == 0){ + Sleep(125); /* ensure 2nd header is done playing before waveOutWrite */ + waveOutWrite(hwo, (WAVEHDR *)dwParam1, sizeof(WAVEHDR)); + } + + break; + } + + g_tid = 0; + SetEvent((HANDLE)dwInstance); +} + +static void test_reentrant_callback(void) +{ + MMRESULT rc; + WAVEHDR hdr[2]; + HWAVEOUT wout; + WAVEFORMATEX fmt; + DWORD wait; + HANDLE hevent; + + if(waveOutGetNumDevs() == 0) + return; + + fmt.wFormatTag = WAVE_FORMAT_PCM; + fmt.nChannels = 2; + fmt.nSamplesPerSec = 44100; + fmt.wBitsPerSample = 16; + fmt.nBlockAlign = fmt.nChannels * fmt.wBitsPerSample / 8; + fmt.nAvgBytesPerSec = fmt.nBlockAlign * fmt.nSamplesPerSec; + fmt.cbSize = sizeof(WAVEFORMATEX); + + hevent = CreateEventW(NULL, FALSE, FALSE, NULL); + g_tid = GetCurrentThreadId(); + + rc = waveOutOpen(&wout, WAVE_MAPPER, &fmt, (DWORD_PTR)test_reentrant_callback_func, + (DWORD_PTR)hevent, CALLBACK_FUNCTION); + ok(rc == MMSYSERR_NOERROR || rc == WAVERR_BADFORMAT || + rc == MMSYSERR_INVALFLAG || rc == MMSYSERR_INVALPARAM, + "waveOutOpen(%s) failed: %s\n", dev_name(WAVE_MAPPER), wave_out_error(rc)); + if(rc != MMSYSERR_NOERROR){ + CloseHandle(hevent); + return; + } + + wait = WaitForSingleObject(hevent, 1000); + ok(wait == WAIT_OBJECT_0, "wave open callback missed\n"); + + memset(hdr, 0, sizeof(hdr)); + hdr[0].dwBufferLength = (fmt.nSamplesPerSec * fmt.nBlockAlign / 10); + hdr[1].dwBufferLength = hdr[0].dwBufferLength; + hdr[1].lpData = hdr[0].lpData = + HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, hdr[0].dwBufferLength); + + rc = waveOutPrepareHeader(wout, &hdr[0], sizeof(hdr[0])); + ok(rc == MMSYSERR_NOERROR, "waveOutPrepareHeader failed: %s\n", wave_out_error(rc)); + + rc = waveOutPrepareHeader(wout, &hdr[1], sizeof(hdr[1])); + ok(rc == MMSYSERR_NOERROR, "waveOutPrepareHeader failed: %s\n", wave_out_error(rc)); + + rc = waveOutWrite(wout, &hdr[0], sizeof(hdr[0])); + ok(rc == MMSYSERR_NOERROR, "waveOutWrite failed: %s\n", wave_out_error(rc)); + + rc = waveOutWrite(wout, &hdr[1], sizeof(hdr[1])); + ok(rc == MMSYSERR_NOERROR, "waveOutWrite failed: %s\n", wave_out_error(rc)); + + wait = WaitForSingleObject(hevent, 1000); + ok(wait == WAIT_OBJECT_0, "header 1 callback missed\n"); + + wait = WaitForSingleObject(hevent, 1000); + ok(wait == WAIT_OBJECT_0, "header 2 callback missed\n"); + + wait = WaitForSingleObject(hevent, 1000); + ok(wait == WAIT_OBJECT_0, "header 3 callback missed\n"); + + g_tid = GetCurrentThreadId(); + rc = waveOutClose(wout); + ok(rc == MMSYSERR_NOERROR, "waveOutClose failed: %s\n", wave_out_error(rc)); + + HeapFree(GetProcessHeap(), 0, hdr[0].lpData); + CloseHandle(hevent); +} + static void create_wav_file(char *temp_file) { WAVEFORMATEX format; @@ -1727,5 +1829,6 @@ START_TEST(wave) wave_out_tests(); test_sndPlaySound(); test_fragmentsize(); + test_reentrant_callback(); test_PlaySound(); } -- 2.35.0

2 1

[PATCH 1/3] wined3d: Handle discarded buffers in buffer_resource_sub_resource_map().
by Zebediah Figura Jan. 27, 2022

Jan. 27, 2022

This is essentially dead code currently; we never leave buffers in a discarded state. Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/wined3d/buffer.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/dlls/wined3d/buffer.c b/dlls/wined3d/buffer.c index b42c7bd919a..e1c1f7aa5fc 100644 --- a/dlls/wined3d/buffer.c +++ b/dlls/wined3d/buffer.c @@ -915,6 +915,7 @@ static HRESULT buffer_resource_sub_resource_map(struct wined3d_resource *resourc struct wined3d_buffer *buffer = buffer_from_resource(resource); struct wined3d_device *device = resource->device; struct wined3d_context *context; + struct wined3d_bo_address addr; unsigned int offset, size; uint8_t *base; LONG count; @@ -927,10 +928,13 @@ static HRESULT buffer_resource_sub_resource_map(struct wined3d_resource *resourc count = ++resource->map_count; - if (buffer->buffer_object) + context = context_acquire(device, NULL, 0); + + wined3d_buffer_get_memory(buffer, context, &addr); + + if (addr.buffer_object) { unsigned int dirty_offset = offset, dirty_size = size; - struct wined3d_bo_address addr; /* DISCARD invalidates the entire buffer, regardless of the specified * offset and size. Some applications also depend on the entire buffer @@ -946,20 +950,13 @@ static HRESULT buffer_resource_sub_resource_map(struct wined3d_resource *resourc || (!(flags & WINED3D_MAP_WRITE) && (buffer->locations & WINED3D_LOCATION_SYSMEM)) || buffer->flags & WINED3D_BUFFER_PIN_SYSMEM) { - if (!(buffer->locations & WINED3D_LOCATION_SYSMEM)) - { - context = context_acquire(device, NULL, 0); - wined3d_buffer_load_location(buffer, context, WINED3D_LOCATION_SYSMEM); - context_release(context); - } + wined3d_buffer_load_location(buffer, context, WINED3D_LOCATION_SYSMEM); if (flags & WINED3D_MAP_WRITE) wined3d_buffer_invalidate_range(buffer, WINED3D_LOCATION_BUFFER, dirty_offset, dirty_size); } else { - context = context_acquire(device, NULL, 0); - if (flags & WINED3D_MAP_DISCARD) wined3d_buffer_validate_location(buffer, WINED3D_LOCATION_BUFFER); else @@ -976,8 +973,6 @@ static HRESULT buffer_resource_sub_resource_map(struct wined3d_resource *resourc if (count == 1) { - addr.buffer_object = buffer->buffer_object; - addr.addr = 0; buffer->map_ptr = wined3d_context_map_bo_address(context, &addr, resource->size, flags); /* We are accessing buffer->resource.client from the CS thread, * but it's safe because the client thread will wait for the @@ -1009,11 +1004,11 @@ static HRESULT buffer_resource_sub_resource_map(struct wined3d_resource *resourc TRACE("New pointer is %p.\n", resource->heap_memory); } } - - context_release(context); } } + context_release(context); + base = buffer->map_ptr ? buffer->map_ptr : resource->heap_memory; *map_ptr = base + offset; -- 2.34.1

5 11

Re: [PATCH] ws2_32: Increased MAX_SOCKETS_PER_PROCESS to 256
by Andrew Miskell Jan. 27, 2022

Jan. 27, 2022

Based on the documentation from Microsoft, modern versions of Windows use Windows Socket version 2.2. This particular parameter which sets the value of iMaxSockets is only used in the Windows Socket 1.0/1.1 implementations and is/should be ignored in all 2.x implementations, which may explain why you’re not seeing this in modern Windows versions. I’ve resubmitted a patch to remove the define and replace the variable used for iMaxSockets.

1 0

[PATCH v2 6/6] msvcp140_atomic_wait: Implement __std_bulk_submit_threadpool_work.
by Daniel Lehman Jan. 27, 2022

Jan. 27, 2022

v2 - remove racy test

1 0

[PATCH v2 5/6] msvcp140_atomic_wait: Implement __std_close_threadpool_work.
by Daniel Lehman Jan. 27, 2022

Jan. 27, 2022

1 0

[PATCH v2 4/6] msvcp140_atomic_wait: Implement __std_wait_for_threadpool_work_callbacks.
by Daniel Lehman Jan. 27, 2022

Jan. 27, 2022

1 0

[PATCH v2 3/6] msvcp140_atomic_wait: Implement __std_submit_threadpool_work.
by Daniel Lehman Jan. 27, 2022

Jan. 27, 2022

1 0