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

Re: [PATCH 0/3] MR225: server: Fix races related to socket errors

by Piotr Caban (＠piotr)

[0001-tests.txt](/uploads/2142824c849655d3e830138d938a9b17/0001-tests.txt) Attaching test that demonstrates the problem. It adds sleeps to wineserver so the error is much more likely to occur. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/225#note_1845

June 10, 2022

[PATCH 6/6] winegstreamer: Leave the wm_reader stream_cs when calling the callbacks.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> They may block and we need to enter the stream_cs to stop the reader. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wm_asyncreader.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/dlls/winegstreamer/wm_asyncreader.c b/dlls/winegstreamer/wm_asyncreader.c index fbfbd38f99e..ef6e445a51b 100644 --- a/dlls/winegstreamer/wm_asyncreader.c +++ b/dlls/winegstreamer/wm_asyncreader.c @@ -99,7 +99,12 @@ static DWORD WINAPI stream_thread(void *arg) QWORD user_time = reader->user_time; if (pts > user_time && reader->reader.callback_advanced) + { + LeaveCriticalSection(&reader->stream_cs); IWMReaderCallbackAdvanced_OnTime(reader->reader.callback_advanced, user_time, reader->context); + EnterCriticalSection(&reader->stream_cs); + } + while (pts > reader->user_time && reader->running) SleepConditionVariableCS(&reader->stream_cv, &reader->stream_cs, INFINITE); } @@ -119,18 +124,23 @@ static DWORD WINAPI stream_thread(void *arg) if (reader->running) { + LeaveCriticalSection(&reader->stream_cs); if (stream->read_compressed) hr = IWMReaderCallbackAdvanced_OnStreamSample(reader->reader.callback_advanced, stream_number, pts, duration, flags, sample, reader->context); else hr = IWMReaderCallback_OnSample(callback, stream_number - 1, pts, duration, flags, sample, reader->context); + EnterCriticalSection(&reader->stream_cs); + TRACE("Callback returned %#lx.\n", hr); } INSSBuffer_Release(sample); } + LeaveCriticalSection(&reader->stream_cs); + if (hr == NS_E_NO_MORE_SAMPLES) { IWMReaderCallback_OnStatus(callback, WMT_END_OF_STREAMING, S_OK, @@ -154,8 +164,6 @@ static DWORD WINAPI stream_thread(void *arg) ERR("Failed to get sample, hr %#lx.\n", hr); } - LeaveCriticalSection(&reader->stream_cs); - TRACE("Reader is stopping; exiting.\n"); return 0; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/226

June 10, 2022

[PATCH 5/6] winegstreamer: Simplify wm_asyncreader stream loop exit code path.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wm_asyncreader.c | 32 +++++++++++------------------ 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/dlls/winegstreamer/wm_asyncreader.c b/dlls/winegstreamer/wm_asyncreader.c index 15e7c2dc9ca..fbfbd38f99e 100644 --- a/dlls/winegstreamer/wm_asyncreader.c +++ b/dlls/winegstreamer/wm_asyncreader.c @@ -102,15 +102,10 @@ static DWORD WINAPI stream_thread(void *arg) IWMReaderCallbackAdvanced_OnTime(reader->reader.callback_advanced, user_time, reader->context); while (pts > reader->user_time && reader->running) SleepConditionVariableCS(&reader->stream_cv, &reader->stream_cs, INFINITE); - if (!reader->running) - { - INSSBuffer_Release(sample); - goto out; - } } else { - for (;;) + while (reader->running) { REFERENCE_TIME current_time = get_current_time(reader); @@ -119,22 +114,20 @@ static DWORD WINAPI stream_thread(void *arg) SleepConditionVariableCS(&reader->stream_cv, &reader->stream_cs, (pts - (current_time - start_time)) / 10000); - - if (!reader->running) - { - INSSBuffer_Release(sample); - goto out; - } } } - if (stream->read_compressed) - hr = IWMReaderCallbackAdvanced_OnStreamSample(reader->reader.callback_advanced, - stream_number, pts, duration, flags, sample, reader->context); - else - hr = IWMReaderCallback_OnSample(callback, stream_number - 1, pts, duration, - flags, sample, reader->context); - TRACE("Callback returned %#lx.\n", hr); + if (reader->running) + { + if (stream->read_compressed) + hr = IWMReaderCallbackAdvanced_OnStreamSample(reader->reader.callback_advanced, + stream_number, pts, duration, flags, sample, reader->context); + else + hr = IWMReaderCallback_OnSample(callback, stream_number - 1, pts, duration, + flags, sample, reader->context); + TRACE("Callback returned %#lx.\n", hr); + } + INSSBuffer_Release(sample); } @@ -161,7 +154,6 @@ static DWORD WINAPI stream_thread(void *arg) ERR("Failed to get sample, hr %#lx.\n", hr); } -out: LeaveCriticalSection(&reader->stream_cs); TRACE("Reader is stopping; exiting.\n"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/226

June 10, 2022

[PATCH 4/6] winegstreamer: Move end conditions out of the wm_asyncreader stream loop.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wm_asyncreader.c | 115 ++++++++++++++-------------- 1 file changed, 56 insertions(+), 59 deletions(-) diff --git a/dlls/winegstreamer/wm_asyncreader.c b/dlls/winegstreamer/wm_asyncreader.c index 97ee1fe60cd..15e7c2dc9ca 100644 --- a/dlls/winegstreamer/wm_asyncreader.c +++ b/dlls/winegstreamer/wm_asyncreader.c @@ -74,12 +74,13 @@ static DWORD WINAPI stream_thread(void *arg) struct async_reader *reader = arg; IWMReaderCallback *callback = reader->callback; REFERENCE_TIME start_time; + struct wm_stream *stream; static const DWORD zero; QWORD pts, duration; WORD stream_number; INSSBuffer *sample; + HRESULT hr = S_OK; DWORD flags; - HRESULT hr; start_time = get_current_time(reader); @@ -88,80 +89,76 @@ static DWORD WINAPI stream_thread(void *arg) while (reader->running) { hr = wm_reader_get_stream_sample(&reader->reader, 0, &sample, &pts, &duration, &flags, &stream_number); + if (hr != S_OK) + break; + + stream = wm_reader_get_stream_by_stream_number(&reader->reader, stream_number); - if (hr == S_OK) + if (reader->user_clock) { - struct wm_stream *stream = wm_reader_get_stream_by_stream_number(&reader->reader, stream_number); + QWORD user_time = reader->user_time; - if (reader->user_clock) + if (pts > user_time && reader->reader.callback_advanced) + IWMReaderCallbackAdvanced_OnTime(reader->reader.callback_advanced, user_time, reader->context); + while (pts > reader->user_time && reader->running) + SleepConditionVariableCS(&reader->stream_cv, &reader->stream_cs, INFINITE); + if (!reader->running) { - QWORD user_time = reader->user_time; - - if (pts > user_time && reader->reader.callback_advanced) - IWMReaderCallbackAdvanced_OnTime(reader->reader.callback_advanced, user_time, reader->context); - while (pts > reader->user_time && reader->running) - SleepConditionVariableCS(&reader->stream_cv, &reader->stream_cs, INFINITE); - if (!reader->running) - { - INSSBuffer_Release(sample); - goto out; - } + INSSBuffer_Release(sample); + goto out; } - else + } + else + { + for (;;) { - for (;;) - { - REFERENCE_TIME current_time = get_current_time(reader); + REFERENCE_TIME current_time = get_current_time(reader); - if (pts <= current_time - start_time) - break; + if (pts <= current_time - start_time) + break; - SleepConditionVariableCS(&reader->stream_cv, &reader->stream_cs, - (pts - (current_time - start_time)) / 10000); + SleepConditionVariableCS(&reader->stream_cv, &reader->stream_cs, + (pts - (current_time - start_time)) / 10000); - if (!reader->running) - { - INSSBuffer_Release(sample); - goto out; - } + if (!reader->running) + { + INSSBuffer_Release(sample); + goto out; } } - - if (stream->read_compressed) - hr = IWMReaderCallbackAdvanced_OnStreamSample(reader->reader.callback_advanced, - stream_number, pts, duration, flags, sample, reader->context); - else - hr = IWMReaderCallback_OnSample(callback, stream_number - 1, pts, duration, - flags, sample, reader->context); - TRACE("Callback returned %#lx.\n", hr); - INSSBuffer_Release(sample); } - else if (hr == NS_E_NO_MORE_SAMPLES) - { - IWMReaderCallback_OnStatus(callback, WMT_END_OF_STREAMING, S_OK, - WMT_TYPE_DWORD, (BYTE *)&zero, reader->context); - IWMReaderCallback_OnStatus(callback, WMT_EOF, S_OK, - WMT_TYPE_DWORD, (BYTE *)&zero, reader->context); - - if (reader->user_clock && reader->reader.callback_advanced) - { - /* We can only get here if user_time is greater than the PTS - * of all samples, in which case we cannot have sent this - * notification already. */ - IWMReaderCallbackAdvanced_OnTime(reader->reader.callback_advanced, - reader->user_time, reader->context); - } - TRACE("Reached end of stream; exiting.\n"); - LeaveCriticalSection(&reader->stream_cs); - return 0; - } + if (stream->read_compressed) + hr = IWMReaderCallbackAdvanced_OnStreamSample(reader->reader.callback_advanced, + stream_number, pts, duration, flags, sample, reader->context); else + hr = IWMReaderCallback_OnSample(callback, stream_number - 1, pts, duration, + flags, sample, reader->context); + TRACE("Callback returned %#lx.\n", hr); + INSSBuffer_Release(sample); + } + + if (hr == NS_E_NO_MORE_SAMPLES) + { + IWMReaderCallback_OnStatus(callback, WMT_END_OF_STREAMING, S_OK, + WMT_TYPE_DWORD, (BYTE *)&zero, reader->context); + IWMReaderCallback_OnStatus(callback, WMT_EOF, S_OK, + WMT_TYPE_DWORD, (BYTE *)&zero, reader->context); + + if (reader->user_clock && reader->reader.callback_advanced) { - ERR("Failed to get sample, hr %#lx.\n", hr); - LeaveCriticalSection(&reader->stream_cs); - return 0; + /* We can only get here if user_time is greater than the PTS + * of all samples, in which case we cannot have sent this + * notification already. */ + IWMReaderCallbackAdvanced_OnTime(reader->reader.callback_advanced, + reader->user_time, reader->context); } + + TRACE("Reached end of stream; exiting.\n"); + } + else if (hr != S_OK) + { + ERR("Failed to get sample, hr %#lx.\n", hr); } out: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/226

June 10, 2022

[PATCH 3/6] winegstreamer: Return E_UNEXPECTED from WMReaderAdvanced2_OpenStream when already opened.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wm_asyncreader.c | 7 +++++++ dlls/wmvcore/tests/wmvcore.c | 3 +++ 2 files changed, 10 insertions(+) diff --git a/dlls/winegstreamer/wm_asyncreader.c b/dlls/winegstreamer/wm_asyncreader.c index e70498b9858..97ee1fe60cd 100644 --- a/dlls/winegstreamer/wm_asyncreader.c +++ b/dlls/winegstreamer/wm_asyncreader.c @@ -725,6 +725,13 @@ static HRESULT WINAPI WMReaderAdvanced2_OpenStream(IWMReaderAdvanced6 *iface, EnterCriticalSection(&reader->reader.cs); + if (reader->reader.wg_parser) + { + LeaveCriticalSection(&reader->reader.cs); + WARN("Stream is already open; returning E_UNEXPECTED.\n"); + return E_UNEXPECTED; + } + if (SUCCEEDED(hr = wm_reader_open_stream(&reader->reader, stream))) open_stream(reader, callback, context); diff --git a/dlls/wmvcore/tests/wmvcore.c b/dlls/wmvcore/tests/wmvcore.c index 59d1f5abcc6..f639184de90 100644 --- a/dlls/wmvcore/tests/wmvcore.c +++ b/dlls/wmvcore/tests/wmvcore.c @@ -2181,6 +2181,9 @@ static void test_async_reader_streaming(void) ret = WaitForSingleObject(callback.got_opened, 1000); ok(!ret, "Wait timed out.\n"); + hr = IWMReaderAdvanced2_OpenStream(advanced, &stream.IStream_iface, &callback.IWMReaderCallback_iface, (void **)0xdeadbee0); + ok(hr == E_UNEXPECTED, "Got hr %#lx.\n", hr); + count = 0xdeadbeef; hr = IWMReader_GetOutputCount(reader, &count); ok(hr == S_OK, "Got hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/226

June 10, 2022

[PATCH 2/6] winegstreamer: Return E_UNEXPECTED from WMReader_Open when already opened.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wm_asyncreader.c | 7 +++++++ dlls/wmvcore/tests/wmvcore.c | 3 +++ 2 files changed, 10 insertions(+) diff --git a/dlls/winegstreamer/wm_asyncreader.c b/dlls/winegstreamer/wm_asyncreader.c index df7c701f6f0..e70498b9858 100644 --- a/dlls/winegstreamer/wm_asyncreader.c +++ b/dlls/winegstreamer/wm_asyncreader.c @@ -222,6 +222,13 @@ static HRESULT WINAPI WMReader_Open(IWMReader *iface, const WCHAR *url, EnterCriticalSection(&reader->reader.cs); + if (reader->reader.wg_parser) + { + LeaveCriticalSection(&reader->reader.cs); + WARN("Stream is already open; returning E_UNEXPECTED.\n"); + return E_UNEXPECTED; + } + if (SUCCEEDED(hr = wm_reader_open_file(&reader->reader, url))) open_stream(reader, callback, context); diff --git a/dlls/wmvcore/tests/wmvcore.c b/dlls/wmvcore/tests/wmvcore.c index 315173b9860..59d1f5abcc6 100644 --- a/dlls/wmvcore/tests/wmvcore.c +++ b/dlls/wmvcore/tests/wmvcore.c @@ -2541,6 +2541,9 @@ static void test_async_reader_file(void) ret = WaitForSingleObject(callback.got_opened, 1000); ok(!ret, "Wait timed out.\n"); + hr = IWMReader_Open(reader, filename, &callback.IWMReaderCallback_iface, (void **)0xdeadbee0); + ok(hr == E_UNEXPECTED, "Got hr %#lx.\n", hr); + count = 0xdeadbeef; hr = IWMReader_GetOutputCount(reader, &count); ok(hr == S_OK, "Got hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/226

June 10, 2022

[PATCH 1/6] winegstreamer: Return NS_E_INVALID_REQUEST on WMReader Start before Open.

by Rémi Bernon

From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wm_asyncreader.c | 7 +++++++ dlls/wmvcore/tests/wmvcore.c | 2 ++ 2 files changed, 9 insertions(+) diff --git a/dlls/winegstreamer/wm_asyncreader.c b/dlls/winegstreamer/wm_asyncreader.c index 947b3307a3a..df7c701f6f0 100644 --- a/dlls/winegstreamer/wm_asyncreader.c +++ b/dlls/winegstreamer/wm_asyncreader.c @@ -318,6 +318,13 @@ static HRESULT WINAPI WMReader_Start(IWMReader *iface, EnterCriticalSection(&reader->reader.cs); + if (!reader->reader.wg_parser) + { + LeaveCriticalSection(&reader->reader.cs); + WARN("No stream is open; returning NS_E_INVALID_REQUEST.\n"); + return NS_E_INVALID_REQUEST; + } + stop_streaming(reader); IWMReaderCallback_OnStatus(reader->callback, WMT_STARTED, S_OK, WMT_TYPE_DWORD, (BYTE *)&zero, context); diff --git a/dlls/wmvcore/tests/wmvcore.c b/dlls/wmvcore/tests/wmvcore.c index 50a7a023488..315173b9860 100644 --- a/dlls/wmvcore/tests/wmvcore.c +++ b/dlls/wmvcore/tests/wmvcore.c @@ -2171,6 +2171,8 @@ static void test_async_reader_streaming(void) hr = IWMReader_Stop(reader); ok(hr == E_UNEXPECTED, "Got hr %#lx.\n", hr); + hr = IWMReader_Start(reader, 0, 0, 1.0, NULL); + ok(hr == NS_E_INVALID_REQUEST, "Got hr %#lx.\n", hr); hr = IWMReaderAdvanced2_OpenStream(advanced, &stream.IStream_iface, &callback.IWMReaderCallback_iface, (void **)0xdeadbeef); ok(hr == S_OK, "Got hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/226

June 10, 2022

[PATCH 0/6] MR226: winegstreamer: Leave stream_cs when calling the async reader callbacks.

by Rémi Bernon

For https://gitlab.winehq.org/wine/wine/-/merge_requests/140 we will want to be able to enter the ASF Reader filter CS in the callbacks, for instance when a sample is received, or to notify about EOS. At the same time, another thread will possibly try to stop the filter, calling `IWMReader_Stop` which will try to enter the stream_cs as well and wait on the stream thread. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/226

June 10, 2022

Re: [PATCH v2 2/4] advapi32/tests: Add some tests for RegRenameKey().

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=116705 Your paranoid android. === w10pro64_ja (64 bit report) === advapi32: registry.c:3471: Test failed: expected ERROR_SUCCESS, got 6 registry.c:3599: Test failed: expected WAIT_OBJECT_0, got 258

June 10, 2022

[PATCH 3/3] server: Don't reset socket error in IOCTL_AFD_GET_SO_ERROR.

by Piotr Caban

From: Piotr Caban <piotr(a)codeweavers.com> Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> --- server/sock.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/sock.c b/server/sock.c index c4bbd388097..466899af064 100644 --- a/server/sock.c +++ b/server/sock.c @@ -2798,7 +2798,6 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) case IOCTL_AFD_WINE_GET_SO_ERROR: { int error; - socklen_t len = sizeof(error); unsigned int i; if (get_reply_max_size() < sizeof(error)) @@ -2807,7 +2806,7 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) return; } - if (getsockopt( unix_fd, SOL_SOCKET, SO_ERROR, (char *)&error, &len ) < 0) + if (sock_error( sock, &error ) < 0) { set_error( sock_get_ntstatus( errno ) ); return; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/225

June 10, 2022