[PATCH v5 0/4] MR3027: wine{alsa,coreaudio,oss,pulse}: Implement main loop logic in mmdevapi.
-- v5: wineoss: Implement main_loop in unixlib. winecoreaudio: Implement main_loop in unixlib. winealsa: Implement main_loop in unixlib. winepulse: Move main loop logic into mmdevapi. https://gitlab.winehq.org/wine/wine/-/merge_requests/3027
From: Davide Beatrici <git(a)davidebeatrici.dev> --- dlls/mmdevapi/client.c | 40 ++++++++++++++++++++++++++++++++ dlls/mmdevapi/main.c | 2 ++ dlls/mmdevapi/mmdevapi_private.h | 2 ++ dlls/winepulse.drv/mmdevdrv.c | 33 +++++--------------------- 4 files changed, 50 insertions(+), 27 deletions(-) diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c index 03bfbabd536..01542f434a8 100644 --- a/dlls/mmdevapi/client.c +++ b/dlls/mmdevapi/client.c @@ -38,6 +38,16 @@ extern void sessions_unlock(void) DECLSPEC_HIDDEN; extern struct audio_session_wrapper *session_wrapper_create(struct audio_client *client) DECLSPEC_HIDDEN; +static HANDLE main_loop_thread; + +void main_loop_stop(void) +{ + if (main_loop_thread) { + WaitForSingleObject(main_loop_thread, INFINITE); + CloseHandle(main_loop_thread); + } +} + void set_stream_volumes(struct audio_client *This) { struct set_volumes_params params; @@ -114,6 +124,36 @@ static void dump_fmt(const WAVEFORMATEX *fmt) } } +static DWORD CALLBACK main_loop_func(void *event) +{ + struct main_loop_params params; + + SetThreadDescription(GetCurrentThread(), L"audio_client_main"); + + params.event = event; + + WINE_UNIX_CALL(main_loop, ¶ms); + + return 0; +} + +HRESULT main_loop_start(void) +{ + if (!main_loop_thread) { + HANDLE event = CreateEventW(NULL, TRUE, FALSE, NULL); + if (!(main_loop_thread = CreateThread(NULL, 0, main_loop_func, event, 0, NULL))) { + CloseHandle(event); + return E_FAIL; + } + + SetThreadPriority(main_loop_thread, THREAD_PRIORITY_TIME_CRITICAL); + WaitForSingleObject(event, INFINITE); + CloseHandle(event); + } + + return S_OK; +} + static DWORD CALLBACK timer_loop_func(void *user) { struct timer_loop_params params; diff --git a/dlls/mmdevapi/main.c b/dlls/mmdevapi/main.c index d3d4ec6a905..676bc53d68d 100644 --- a/dlls/mmdevapi/main.c +++ b/dlls/mmdevapi/main.c @@ -196,6 +196,8 @@ BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved) DisableThreadLibraryCalls(hinstDLL); break; case DLL_PROCESS_DETACH: + main_loop_stop(); + if (drvs.module_unixlib) { const NTSTATUS status = __wine_unix_call(drvs.module_unixlib, process_detach, NULL); if (status) diff --git a/dlls/mmdevapi/mmdevapi_private.h b/dlls/mmdevapi/mmdevapi_private.h index 684d303eefb..5abde115951 100644 --- a/dlls/mmdevapi/mmdevapi_private.h +++ b/dlls/mmdevapi/mmdevapi_private.h @@ -80,4 +80,6 @@ extern HRESULT SpatialAudioClient_Create(IMMDevice *device, ISpatialAudioClient extern HRESULT load_devices_from_reg(void) DECLSPEC_HIDDEN; extern HRESULT load_driver_devices(EDataFlow flow) DECLSPEC_HIDDEN; +extern void main_loop_stop(void) DECLSPEC_HIDDEN; + extern const WCHAR drv_keyW[] DECLSPEC_HIDDEN; diff --git a/dlls/winepulse.drv/mmdevdrv.c b/dlls/winepulse.drv/mmdevdrv.c index 12e36fae827..60a660dcf8b 100644 --- a/dlls/winepulse.drv/mmdevdrv.c +++ b/dlls/winepulse.drv/mmdevdrv.c @@ -56,7 +56,6 @@ WINE_DEFAULT_DEBUG_CHANNEL(pulse); #define NULL_PTR_ERR MAKE_HRESULT(SEVERITY_ERROR, FACILITY_WIN32, RPC_X_NULL_REF_POINTER) -static HANDLE pulse_thread; static struct list g_sessions = LIST_INIT(g_sessions); static struct list g_devices_cache = LIST_INIT(g_devices_cache); @@ -115,11 +114,6 @@ BOOL WINAPI DllMain(HINSTANCE dll, DWORD reason, void *reserved) LIST_FOR_EACH_ENTRY_SAFE(device, device_next, &g_devices_cache, struct device_cache, entry) free(device); - - if (pulse_thread) { - WaitForSingleObject(pulse_thread, INFINITE); - CloseHandle(pulse_thread); - } } return TRUE; } @@ -134,6 +128,8 @@ extern const IAudioClockVtbl AudioClock_Vtbl; extern const IAudioClock2Vtbl AudioClock2_Vtbl; extern const IAudioStreamVolumeVtbl AudioStreamVolume_Vtbl; +extern HRESULT main_loop_start(void) DECLSPEC_HIDDEN; + extern struct audio_session_wrapper *session_wrapper_create( struct audio_client *client) DECLSPEC_HIDDEN; @@ -157,15 +153,6 @@ static void pulse_release_stream(stream_handle stream, HANDLE timer) pulse_call(release_stream, ¶ms); } -static DWORD CALLBACK pulse_mainloop_thread(void *event) -{ - struct main_loop_params params; - params.event = event; - SetThreadDescription(GetCurrentThread(), L"winepulse_mainloop"); - pulse_call(main_loop, ¶ms); - return 0; -} - typedef struct tagLANGANDCODEPAGE { WORD wLanguage; @@ -735,19 +722,11 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, return AUDCLNT_E_ALREADY_INITIALIZED; } - if (!pulse_thread) + if (FAILED(hr = main_loop_start())) { - HANDLE event = CreateEventW(NULL, TRUE, FALSE, NULL); - if (!(pulse_thread = CreateThread(NULL, 0, pulse_mainloop_thread, event, 0, NULL))) - { - ERR("Failed to create mainloop thread.\n"); - sessions_unlock(); - CloseHandle(event); - return E_FAIL; - } - SetThreadPriority(pulse_thread, THREAD_PRIORITY_TIME_CRITICAL); - WaitForSingleObject(event, INFINITE); - CloseHandle(event); + ERR("Failed to create mainloop thread.\n"); + sessions_unlock(); + return hr; } params.name = name = get_application_name(TRUE); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3027
From: Davide Beatrici <git(a)davidebeatrici.dev> --- dlls/winealsa.drv/alsa.c | 24 ++++++++++++++++++++++-- dlls/winealsa.drv/mmdevdrv.c | 8 ++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/dlls/winealsa.drv/alsa.c b/dlls/winealsa.drv/alsa.c index dbf85a7947e..4930850e9dc 100644 --- a/dlls/winealsa.drv/alsa.c +++ b/dlls/winealsa.drv/alsa.c @@ -476,6 +476,13 @@ static WCHAR *alsa_get_card_name(int card) return ret; } +static NTSTATUS alsa_main_loop(void *args) +{ + struct main_loop_params *params = args; + NtSetEvent(params->event, NULL); + return STATUS_SUCCESS; +} + static NTSTATUS alsa_get_endpoint_ids(void *args) { static const WCHAR defaultW[] = {'d','e','f','a','u','l','t',0}; @@ -2493,7 +2500,7 @@ unixlib_entry_t __wine_unix_call_funcs[] = { alsa_not_implemented, alsa_not_implemented, - alsa_not_implemented, + alsa_main_loop, alsa_get_endpoint_ids, alsa_create_stream, alsa_release_stream, @@ -2531,6 +2538,19 @@ unixlib_entry_t __wine_unix_call_funcs[] = typedef UINT PTR32; +static NTSTATUS alsa_wow64_main_loop(void *args) +{ + struct + { + PTR32 event; + } *params32 = args; + struct main_loop_params params = + { + .event = ULongToHandle(params32->event) + }; + return alsa_main_loop(¶ms); +} + static NTSTATUS alsa_wow64_get_endpoint_ids(void *args) { struct @@ -2934,7 +2954,7 @@ unixlib_entry_t __wine_unix_call_wow64_funcs[] = { alsa_not_implemented, alsa_not_implemented, - alsa_not_implemented, + alsa_wow64_main_loop, alsa_wow64_get_endpoint_ids, alsa_wow64_create_stream, alsa_wow64_release_stream, diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c index 7c7096d584a..a629b47a742 100644 --- a/dlls/winealsa.drv/mmdevdrv.c +++ b/dlls/winealsa.drv/mmdevdrv.c @@ -76,6 +76,8 @@ extern const IAudioClock2Vtbl AudioClock2_Vtbl; extern const IAudioStreamVolumeVtbl AudioStreamVolume_Vtbl; extern const IChannelAudioVolumeVtbl ChannelAudioVolume_Vtbl; +extern HRESULT main_loop_start(void) DECLSPEC_HIDDEN; + extern struct audio_session_wrapper *session_wrapper_create( struct audio_client *client) DECLSPEC_HIDDEN; @@ -598,6 +600,12 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, return AUDCLNT_E_ALREADY_INITIALIZED; } + if(FAILED(params.result = main_loop_start())){ + ERR("Failed to create mainloop thread.\n"); + sessions_unlock(); + return params.result; + } + dump_fmt(fmt); params.name = NULL; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3027
From: Davide Beatrici <git(a)davidebeatrici.dev> --- dlls/winecoreaudio.drv/coreaudio.c | 24 ++++++++++++++++++++++-- dlls/winecoreaudio.drv/mmdevdrv.c | 8 ++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/dlls/winecoreaudio.drv/coreaudio.c b/dlls/winecoreaudio.drv/coreaudio.c index d031b82583e..f41118e62f5 100644 --- a/dlls/winecoreaudio.drv/coreaudio.c +++ b/dlls/winecoreaudio.drv/coreaudio.c @@ -196,6 +196,13 @@ static BOOL device_has_channels(AudioDeviceID device, EDataFlow flow) return ret; } +static NTSTATUS unix_main_loop(void *args) +{ + struct main_loop_params *params = args; + NtSetEvent(params->event, NULL); + return STATUS_SUCCESS; +} + static NTSTATUS unix_get_endpoint_ids(void *args) { struct get_endpoint_ids_params *params = args; @@ -1775,7 +1782,7 @@ unixlib_entry_t __wine_unix_call_funcs[] = { unix_not_implemented, unix_not_implemented, - unix_not_implemented, + unix_main_loop, unix_get_endpoint_ids, unix_create_stream, unix_release_stream, @@ -1813,6 +1820,19 @@ unixlib_entry_t __wine_unix_call_funcs[] = typedef UINT PTR32; +static NTSTATUS unix_wow64_main_loop(void *args) +{ + struct + { + PTR32 event; + } *params32 = args; + struct main_loop_params params = + { + .event = ULongToHandle(params32->event) + }; + return unix_main_loop(¶ms); +} + static NTSTATUS unix_wow64_get_endpoint_ids(void *args) { struct @@ -2157,7 +2177,7 @@ unixlib_entry_t __wine_unix_call_wow64_funcs[] = { unix_not_implemented, unix_not_implemented, - unix_not_implemented, + unix_wow64_main_loop, unix_wow64_get_endpoint_ids, unix_wow64_create_stream, unix_wow64_release_stream, diff --git a/dlls/winecoreaudio.drv/mmdevdrv.c b/dlls/winecoreaudio.drv/mmdevdrv.c index f268e9565b4..52e27fb8d4e 100644 --- a/dlls/winecoreaudio.drv/mmdevdrv.c +++ b/dlls/winecoreaudio.drv/mmdevdrv.c @@ -70,6 +70,8 @@ static CRITICAL_SECTION_DEBUG g_sessions_lock_debug = static CRITICAL_SECTION g_sessions_lock = { &g_sessions_lock_debug, -1, 0, 0, 0, 0 }; static struct list g_sessions = LIST_INIT(g_sessions); +extern HRESULT main_loop_start(void) DECLSPEC_HIDDEN; + extern struct audio_session_wrapper *session_wrapper_create( struct audio_client *client) DECLSPEC_HIDDEN; @@ -587,6 +589,12 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, return AUDCLNT_E_ALREADY_INITIALIZED; } + if(FAILED(params.result = main_loop_start())){ + ERR("Failed to create mainloop thread.\n"); + sessions_unlock(); + return params.result; + } + params.name = NULL; params.device = This->device_name; params.flow = This->dataflow; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3027
From: Davide Beatrici <git(a)davidebeatrici.dev> --- dlls/wineoss.drv/mmdevdrv.c | 8 ++++++++ dlls/wineoss.drv/oss.c | 24 ++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/dlls/wineoss.drv/mmdevdrv.c b/dlls/wineoss.drv/mmdevdrv.c index 12f10fd4377..7c3be094143 100644 --- a/dlls/wineoss.drv/mmdevdrv.c +++ b/dlls/wineoss.drv/mmdevdrv.c @@ -82,6 +82,8 @@ extern const IAudioClock2Vtbl AudioClock2_Vtbl; extern const IAudioStreamVolumeVtbl AudioStreamVolume_Vtbl; extern const IChannelAudioVolumeVtbl ChannelAudioVolume_Vtbl; +extern HRESULT main_loop_start(void) DECLSPEC_HIDDEN; + extern struct audio_session_wrapper *session_wrapper_create( struct audio_client *client) DECLSPEC_HIDDEN; @@ -575,6 +577,12 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, return AUDCLNT_E_ALREADY_INITIALIZED; } + if(FAILED(params.result = main_loop_start())){ + ERR("Failed to create mainloop thread.\n"); + sessions_unlock(); + return params.result; + } + params.name = NULL; params.device = This->device_name; params.flow = This->dataflow; diff --git a/dlls/wineoss.drv/oss.c b/dlls/wineoss.drv/oss.c index df03d3286d9..5049c711e98 100644 --- a/dlls/wineoss.drv/oss.c +++ b/dlls/wineoss.drv/oss.c @@ -221,6 +221,13 @@ static void get_default_device(EDataFlow flow, char device[OSS_DEVNODE_SIZE]) return; } +static NTSTATUS oss_main_loop(void *args) +{ + struct main_loop_params *params = args; + NtSetEvent(params->event, NULL); + return STATUS_SUCCESS; +} + static NTSTATUS oss_get_endpoint_ids(void *args) { struct get_endpoint_ids_params *params = args; @@ -1684,7 +1691,7 @@ unixlib_entry_t __wine_unix_call_funcs[] = { oss_not_implemented, oss_not_implemented, - oss_not_implemented, + oss_main_loop, oss_get_endpoint_ids, oss_create_stream, oss_release_stream, @@ -1738,6 +1745,19 @@ static NTSTATUS oss_wow64_test_connect(void *args) return STATUS_SUCCESS; } +static NTSTATUS oss_wow64_main_loop(void *args) +{ + struct + { + PTR32 event; + } *params32 = args; + struct main_loop_params params = + { + .event = ULongToHandle(params32->event) + }; + return oss_main_loop(¶ms); +} + static NTSTATUS oss_wow64_get_endpoint_ids(void *args) { struct @@ -2108,7 +2128,7 @@ unixlib_entry_t __wine_unix_call_wow64_funcs[] = { oss_not_implemented, oss_not_implemented, - oss_not_implemented, + oss_wow64_main_loop, oss_wow64_get_endpoint_ids, oss_wow64_create_stream, oss_wow64_release_stream, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3027
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=133645 Your paranoid android. === debian11 (32 bit report) === user32: msg.c:6897: Test failed: SetFocus(hwnd) on a button: 4: the msg 0x0007 was expected, but got msg 0x0005 instead msg.c:6897: Test failed: SetFocus(hwnd) on a button: 5: the msg 0x0135 was expected, but got msg 0x030f instead msg.c:6897: Test failed: SetFocus(hwnd) on a button: 6: the msg 0x0111 was expected, but got msg 0x001c instead msg.c:6897: Test failed: SetFocus(hwnd) on a button: 7: the msg 0x8000 was expected, but got msg 0x0086 instead msg.c:6897: Test failed: SetFocus(hwnd) on a button: 8: the msg sequence is not complete: expected 0000 - actual 0006
Davide Beatrici (@davidebeatrici) commented about dlls/winepulse.drv/mmdevdrv.c:
- if (!pulse_thread) + if (FAILED(hr = main_loop_start())) { - HANDLE event = CreateEventW(NULL, TRUE, FALSE, NULL); - if (!(pulse_thread = CreateThread(NULL, 0, pulse_mainloop_thread, event, 0, NULL))) - { - ERR("Failed to create mainloop thread.\n"); - sessions_unlock(); - CloseHandle(event); - return E_FAIL; - } - SetThreadPriority(pulse_thread, THREAD_PRIORITY_TIME_CRITICAL); - WaitForSingleObject(event, INFINITE); - CloseHandle(event); + ERR("Failed to create mainloop thread.\n");
Should this perhaps be moved into `main_loop_start()`? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3027#note_35313
Huw Davies (@huw) commented about dlls/mmdevapi/main.c:
DisableThreadLibraryCalls(hinstDLL); break; case DLL_PROCESS_DETACH: + main_loop_stop(); +
Doesn't this cause a deadlock? This call will block until the main loop thread terminates, but that thread is not told to terminate until the process_detach syscall, which is called later on. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3027#note_35433
Should this perhaps be moved into `main_loop_start()`?
Yes, that would be better. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3027#note_35434
On Mon Jun 12 10:34:31 2023 +0000, Huw Davies wrote:
Doesn't this cause a deadlock? This call will block until the main loop thread terminates, but that thread is not told to terminate until the process_detach syscall, which is called later on. Good catch! Not sure why it doesn't happen in practice...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3027#note_35491
participants (4)
-
Davide Beatrici -
Davide Beatrici (＠davidebeatrici)
-
Huw Davies (＠huw)
-
Marvin