[Bug 59522] New: explorerframe: ITaskbarList3 SetProgressValue/SetProgressState stubs - could use native desktop backends
http://bugs.winehq.org/show_bug.cgi?id=59522 Bug ID: 59522 Summary: explorerframe: ITaskbarList3 SetProgressValue/SetProgressState stubs - could use native desktop backends Product: Wine Version: 11.4 Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: explorerframe Assignee: wine-bugs@list.winehq.org Reporter: Robert-Gerigk@online.de Distribution: --- ITaskbarList3::SetProgressValue() and SetProgressState() in dlls/explorerframe/taskbarlist.c are currently stubs (FIXME + return S_OK). This causes Issues with .NET/WPF applications that use the Windows 7+ taskbar progress API. Some appplications (e.g. WPF apps using TaskbarItemInfo) deadlock or behave incorrectly when the COM call chain around these methods interacts with the UI thread. Wine already implements plattform-specific desktop integration in several places: - macOS: Full Cocoa integration (NSStatusBar, NSDockTile) in winemac.drv - X11: EWMH hints including _NET_WM_STATE_DEMANDS_ATTENTION in winex11.drv - Wayland: xdg-toplevel-icon-v1 protocol in winewayland.drv - DBus: Production clients for BlueZ and UDisks in winebth.sys and mountmgr.sys A natural Extension would be implementing taskbar progress via the com.canonical.Unity.LauncherEntry DBus interface, wich is supported by KDE Plasma, GNOME, and other Linux desktop environments. Firefox and LibreOffice already use this interface for native taskbar progress. Proposed mapping: TBPF_NOPROGRESS -> progress-visible: false TBPF_NORMAL -> progress-visible: true, progress: value TBPF_INDETERMINATE -> progress-visible: true, progress: 0.0 TBPF_ERROR -> urgent: true TBPF_PAUSED -> progress-visible: true (no direct equivalent) The implementation would follow the existing DBus pattern from mountmgr.sys/dbus.c (dlopen/dlsym, graceful degradation when libdbus-1 is unavailable). I'd be willing to work on a Linux/DBus patch if there's interest. Looking for feedback on: 1. Is this the right component (explorerframe) or should this be handled in the platform drivers (winex11.drv, winewayland.drv)? 2. How should the .desktop file association be resolved? (DBus needs an application:// URI) 3. Any concerns about adding a libdbus-1 dependency to explorerframe? I'm currently working on getting KNX ETS 6.4 (a .NET/WPF application) running under Wine and ran into several issues along teh way, this being one of them. The application uses TaskbarItemInfo for progress display wich leads to Problems in the COM/UI thread interaction. ETS 6.4 does run on Linux now, but only trough launch scripts, .dll patches, registry workarounds, and a heavily patched Wine prefix (winetricks dotnet48 + manual fixes) - it would be great to reduce those workarounds over time. I am mostly a solo devloper with an electrical engineering background and fairly new to collaborative development on Gitlab/GitHub, so I'd appreciate any Guidance on the right approach for this - happy to learn. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59522 Esme Povirk <madewokherd@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |madewokherd@gmail.com --- Comment #1 from Esme Povirk <madewokherd@gmail.com> --- It's not clear to me why a stub returning S_OK would cause a deadlock. If the problem is COM marshaling, changing the implementation won't affect that. You might just need to switch the object to a free-threaded model. I think the desktop integration will have to live in the user driver, as I don't see how else one could properly map an HWND to whatever object needs to be adjusted, without baking in the assumption that it has something to do with .desktop shortcuts. Mapping an HWND to an "application" is a much larger problem that we don't currently have a solution for. Windows has a concept of "App User Model IDs" (https://learn.microsoft.com/en-us/windows/win32/shell/appids) which are string values that identify an "application". I don't think these are exposed through the Windows API, so we can't test them directly. This ID can then be mapped to a .lnk shortcut, or a shortcut can be created when a user decides to "pin" an application to the start menu or taskbar. Last I looked into this, Linux desktops used a daemon called "BAMF" (https://launchpad.net/bamf) to do something similar. IMO, the ideal is for win32u to know the App User Model ID of each window and send it to the user driver, which would use that to map HWND to .desktop file and pass that information along to BAMF. But this would be a big project, and I doubt it's needed for your purposes. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59522 --- Comment #2 from Esme Povirk <madewokherd@gmail.com> --- There's also a possibility that Wine Mono is at fault here. The COM code is incomplete and doesn't correctly handle accessing the same COM interface from multiple threads. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59522 --- Comment #3 from Robert-Gerigk@online.de --- Thanks for the fast feedback Esme. Regarding Wine Mono: In my setup, Wine Mono is completlly removed. I disable mscoree during wineboot (WINEDLLOVERRIDES="mscoree=") to prevent Mono installation, then install the real Microsoft .NET 4.8 via winetricks dotnet48 and set *mscoree=native,builtin. The application runs on the native Microsoft CLR, not Wine Mono. The deadlock still occurs in this configuration, so it seems to be in Wine's COM infrastructure rather than Mono's threading code. The specific call chain that triggers it: 1. TaskbarProgressManager.Initialize() calls CoCreateInstance(CLSID_TaskbarList) to obtain ITaskbarList3 2. The COM apartment marshalling between threads blocks indefinitly 3. A second deadlock vector: Wizard ViewModel Dispose() methods call back into TaskbarProgressManager to clean up the progress state. 4. A third vector: async state machine MoveNext handler propagates the resulting exception, wich causes a crash My current workaround patches 30+ methods in the application DLL to return immediately (IL opcode 0x2A = ret), completly neutralizing the TaskbarProgressManager. Regarding the Architecture: makes sense to put this in the user driver. For the HWND-to-application mapping: I'm not familiar with BAMF in detail. Does Wine already interact with BAMF or similar window-matching daemons on the desktop side? If not, would matching the process executable name against installed .desktop files work as a simpler first step? That would cover the common Case where one .desktop file launches one Wine application. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59522 --- Comment #4 from Esme Povirk <madewokherd@gmail.com> --- Hang on, CoCreateInstance is deadlocking? That's weird. I don't see why that would require any marshaling between threads at all (unless .NET Framework is doing that implicitly, in which case I think you probably do need to change the interface to free-threaded). Wine does not currently interact directly with BAMF. Defaulting to an App User Model ID equal to the full path of the main exe would be a reasonable first step, I think, but we'd want the architecture to build on so we can add other rules. I still don't think this will solve any of your deadlocks though. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59522 --- Comment #5 from Robert-Gerigk@online.de --- Thanks for the pointer on the threading model, Esme. That was the key. I tested two changes against ETS 6.4.0 (KNX building automation, .NET/WPF application using Microsoft.WindowsAPICodePack.Taskbar): Change 1: explorerframe.idl Changed the TaskbarList coclass threading model from "apartment" to "both": - threading(apartment), + threading(both), This eliminates the COM cross-apartment marshalling that caused the deadlock in TaskbarProgressManager.Initialize(). The CoCreateInstance call for CLSID_TaskbarList now completes without blocking. Result: 9 of 11 binary patches in the application DLL became unnecessary (Initialize, set_Progress, set_State, SetState, 3 obfuscated TaskbarProgressManager methods, 3 Dispose methods). Change 2: shcore/main.c Implemented Set/GetCurrentProcessExplicitAppUserModelID (were stubs): static WCHAR *explicit_app_user_model_id; SetCurrentProcessExplicit...: stores the appid via CoTaskMemAlloc GetCurrentProcessExplicit...: returns a copy, or E_FAIL if unset The previous stub returned *appid = NULL / E_NOTIMPL, wich caused "String argument cannot be null or empty" when the application's TaskbarManager tried to read back the ApplicationId it had set. Result: the remaining 2 patches became unnecessary (the obfuscated method accessing ApplicationId, and a tamper check). Combined results: With both changes, all 12 binary patches in the application are eliminated (11 method patches in a ViewModel DLL + 1 anti-tamper bypass in the executable). The application DLLs remain completly unmodified and the integrity check passes on its own. Testing was done by systematically restoring original bytes one patch at a time using a toggle script, confirming each method individually. I would be happy to prepare proper patches for both changes if this approach looks right. The shcore change is straightforward, the explorerframe threading change might need more thought regarding thread safety of the current stub Implementation (though the methods only return S_OK with no shared state). -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59522 --- Comment #6 from Esme Povirk <madewokherd@gmail.com> --- Yay, that all sounds right to me. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59522 --- Comment #7 from Esme Povirk <madewokherd@gmail.com> --- OK, I checked on Windows, and they have CLSID_TaskbarList set to apartment, so I guess that part isn't right. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59522 --- Comment #8 from Esme Povirk <madewokherd@gmail.com> --- I wonder if the thread calling CoCreateInstance is in a multi-threaded apartment and should be in a single-threaded apartment for some reason. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59522 --- Comment #9 from Robert-Gerigk@online.de --- I think I found the root cause. The application uses Microsoft.WindowsAPICodePack.Shell.dll wich has a TaskbarList.get_Instance() property with double-check locking: lock (_syncLock) { _taskbarList = new CTaskbarList(); // CoCreateInstance _taskbarList.HrInit(); } This is called from a thread pool thread (MTA). Since CTaskbarList is apartment-threaded, CoCreateInstance needs to marshal to the STA thread. But the STA thread is blocked (waiting for the Async operation to complete), causing the deadlock. On Windows this presumably works because the STA message pump handles the COM marshaling request even while waiting. Under Wine the STA thread doesnt pump messages during the wait, so the marshal request never completes. So the real fix would be in Wine COM message pumping during STA waits, not in the threading model of TaskbarList. The threading(both) workaround avoids the cross-apartment marshal entirely, wich is why it works but is not correct. I will double-check this analysis to make sure it is really the root cause. If it is, a proper fix seems quite complex to me, it appears to be a lot more involved in the COM message pumping during STA blocking waits. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59522 --- Comment #10 from Esme Povirk <madewokherd@gmail.com> --- I don't think a C# lock statement is supposed to run a message loop. I think it just blocks. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59522 Robert-Gerigk@online.de changed: What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #11 from Robert-Gerigk@online.de --- The COM threading deadlock in ITaskbarList3 was fixed upstream in Wine 11.5. The remaining shcore Set/GetCurrentProcessExplicitAppUserModelID part was tracked as bug 59625 and is fixed in Wine 11.7 (MR !10565). FIXED in 11.5 / 11.7. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=59522 Alexandre Julliard <julliard@winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #12 from Alexandre Julliard <julliard@winehq.org> --- Closing bugs fixed in 11.10. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
WineHQ Bugzilla