[PATCH v2 0/1] MR6908: win32u: Allow unsetting the user driver.
The wayland driver does this during initialization. This fixes a regression when running wine without a desktop. -- v2: win32u: Allow unsetting the user driver. https://gitlab.winehq.org/wine/wine/-/merge_requests/6908
From: Sven Baars <sbaars(a)codeweavers.com> The wayland driver does this during initialization. --- dlls/win32u/driver.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/dlls/win32u/driver.c b/dlls/win32u/driver.c index 12390c026ce..e2ef78f1818 100644 --- a/dlls/win32u/driver.c +++ b/dlls/win32u/driver.c @@ -1314,8 +1314,16 @@ void __wine_set_user_driver( const struct user_driver_funcs *funcs, UINT version return; } + if (!funcs) + { + prev = InterlockedExchangePointer( (void **)&user_driver, (void *)&lazy_load_driver ); + if (prev != &lazy_load_driver) + free( prev ); + return; + } + driver = malloc( sizeof(*driver) ); - *driver = funcs ? *funcs : null_user_driver; + *driver = *funcs; #define SET_USER_FUNC(name) \ do { if (!driver->p##name) driver->p##name = nulldrv_##name; } while(0) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6908
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=149978 Your paranoid android. === debian11b (64 bit WoW report) === mfmediaengine: mfmediaengine.c:2564: Test failed: Unexpected time 0.000000. user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000010700E2, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032 Report validation errors: d3d11:d3d11 crashed (c0000005)
This looks okay, although I'm not sure we should actually allow unsetting the driver. Maybe calling `wl_display_connect` earlier would be enough? This change here should work as intended within explorer, because it loads the drivers individually, triggering their unixlib init first, which would call `__wine_set_user_driver( driver_funcs )` and eventually in wayland case `__wine_set_user_driver( NULL )`. In other processes however, user driver loading is triggered from various win32u calls, which waits for explorer to select a driver (written in the registry), then loads them. If this fails at this point (because wayland is unreachable from the context of this process, regardless of what explorer.exe decided), I think `__wine_set_user_driver( &null_user_driver, WINE_GDI_DRIVER_VERSION )` is called regardless of what loading the user driver did. This means that in each case we get a different end result (in the first case we get back to lazydrv, in the second we get nulldrv funcs). If we decide to do this maybe we should at least make it consitent? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6908#note_89082
The problem seems to be twofold. The first problem is that `__wine_set_user_driver( NULL )` doesn't do anything because of `prev = InterlockedCompareExchangePointer( (void **)&user_driver, driver, (void *)&lazy_load_driver );`. We could replace that with ``` if (!funcs) prev = InterlockedExchangePointer( (void **)&user_driver, driver ); else prev = InterlockedCompareExchangePointer( (void **)&user_driver, driver, (void *)&lazy_load_driver ); ``` but then we run into a second problem, which seems to be that we never call `load_driver()` before using the driver. I can fix this issue by adding ``` if (!funcs) send_message( get_desktop_window(), WM_NULL, 0, 0 ); ``` to `__wine_set_user_driver` (this is usually done in `load_desktop_driver`). I'm unsure of how exactly that fixes the issue though. Setting the driver back to the `lazy_load_driver` seemed like a safer option. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6908#note_89099
On Thu Nov 28 10:51:44 2024 +0000, Sven Baars wrote:
The problem seems to be twofold. The first problem is that `__wine_set_user_driver( NULL )` doesn't do anything because of `prev = InterlockedCompareExchangePointer( (void **)&user_driver, driver, (void *)&lazy_load_driver );`. We could replace that with ``` if (!funcs) prev = InterlockedExchangePointer( (void **)&user_driver, driver ); else prev = InterlockedCompareExchangePointer( (void **)&user_driver, driver, (void *)&lazy_load_driver ); ``` but then we run into a second problem, which seems to be that we never call `load_driver()` before using the driver. I can fix this issue by adding ``` if (!funcs) send_message( get_desktop_window(), WM_NULL, 0, 0 ); ``` to `__wine_set_user_driver` (this is usually done in `load_desktop_driver`). I'm unsure of how exactly that fixes the issue though. Setting the driver back to the `lazy_load_driver` seemed like a safer option. Note that right after setting the driver back to the lazy driver, it is still set to the null driver, but then following the usual `load_driver() -> load_desktop_driver()` path.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6908#note_89100
On Thu Nov 28 10:55:18 2024 +0000, Sven Baars wrote:
Note that in my commit, right after setting the driver back to the lazy driver, it is still set to the null driver, but then following the usual `load_driver() -> load_desktop_driver()` path. Alright, seems okay then but I would probably still consider simply moving `wayland_connect` instead.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6908#note_89131
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6908
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6908
participants (5)
-
Huw Davies (＠huw) -
Marvin -
Rémi Bernon
-
Sven Baars
-
Sven Baars (＠sbaars)