[PATCH v14 0/4] MR6315: plugplay: Rework how PnP notifications are sent over RPC to support DBT_DEVTYP_HANDLE events.

Vibhav Pant (＠vibhavp)

20 Aug 2024 20 Aug '24

4:45 p.m.

PnP device notifications/events are currently sent and received from the `plugplay` service as the raw-bytes for the `DBT_DEVTYP_DEVICEINTERFACE` struct that needs to be sent from the PnP code in `ntoskrnl.exe`. While simple, it does not support sending `DBT_DEVTYP_HANDLE` events, as they also include a `HANDLE` to the device the event comes from, which cannot simply be sent as raw bytes. This MR reworks how notifications are sent and received from the `plugplay` service, by: 1. Introducing new structs `DEVICE_BROADCAST_HANDLE` and `DEVICE_BROADCAST_DEVICEINTERFACE` in `plugplay.idl`. The structs are similar to their counterparts in `dbt.h` without the `devicetype` and `size` fields added for the header. 2. Re-write `plugplay_{send,get}_event` to use the newly added union `DEVICE_BROADCAST`, which uses the `devicetype` value as the discriminant. A deep copy of this union is then appended to the event queue of every registered listener. 3. Re-write `I_ScRegisterDeviceNotification` and `device_notify_proc` in `sechost/service.c` to use the newly added types instead. Additionally, add support for registering for `DBT_DEVTYPE_HANDLE` notifications, for which the `OBJECT_NAME_INFORMATION` value for the device `HANDLE` is used to filter notifications in order to correctly dispatch them to their registered callbacks. 4. Re-write the callbacks in `user32/input.c` to accept the `DEVICE_BROADCAST` union instead, and construct the appropriate `DEV_BROADCAST_*` struct from it, which gets sent to `SendMessageTimeoutW` as usual. Support for `DBT_DEVTYP_HANDLE` is needed for the Bluetooth driver stack I have been working on recently, which uses `DBT_DEVTYP_HANDLE` events to notify userspace about newly found Bluetooth devices during discovery and incoming authentication requests, as documented by MS [here](https://learn.microsoft.com/en-us/windows/win32/bluetooth/bluetooth-and-wm-d...). -- v14: plugplay: Broadcast PnP events on a dedicated thread. https://gitlab.winehq.org/wine/wine/-/merge_requests/6315

Show replies by date

Vibhav Pant

20 Aug 20 Aug

4:45 p.m.

New subject: [PATCH v14 1/4] include/wine/plugplay: Separate device broadcast event types into their own structs.

From: Vibhav Pant <vibhavp(a)gmail.com> Instead of sending and receiving events from plugplay as single array of bytes, introduce a new union, DEVICE_BROADCAST. The union currently supports the types DEVICE_BROADCAST_HANDLE and DEVICE_BROADCAST_DEVICEINTERFACE for the DBT_DEVTYP_HANDLE and DBT_DEVTYP_DEVICEINTERFACE event codes respectively, and define individual fields corresponding to their equivalent structs in dbt.h. While this introduces some additional complexity while converting from a DEVICE_BROADCAST to a DEV_BROADCAST_* type, it also allows support for DBT_DEVTYP_HANDLE events to be sent, which was not supported with the previous raw-bytes method, as the DEV_BROADCAST_HANDLE struct was identified with a HANDLE to the originating device file. --- include/wine/plugplay.idl | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/include/wine/plugplay.idl b/include/wine/plugplay.idl index a3e7b04bf30..d5aaebe388e 100644 --- a/include/wine/plugplay.idl +++ b/include/wine/plugplay.idl @@ -1,5 +1,6 @@ /* * Copyright (C) 2020 Zebediah Figura for CodeWeavers + * Copyright (C) 2024 Vibhav Pant * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -18,6 +19,42 @@ import "wtypes.idl"; +typedef enum _broadcast_device_type +{ + /* DBT_DEVTYP_DEVICEINTERFACE */ + BROADCAST_DEVICE_TYPE_DEVICEINTERFACE = 5, + /* DBT_DEVTYP_HANDLE */ + BROADCAST_DEVICE_TYPE_HANDLE = 6 +} broadcast_device_type; + +typedef struct _device_broadcast_header +{ + DWORD reserved; +} device_broadcast_header; + +typedef struct _device_broadcast_deviceinterface +{ + device_broadcast_header header; + GUID class_guid; + [string] LPWSTR name; +} device_broadcast_deviceinterface; + +typedef struct _device_broadcast_handle +{ + device_broadcast_header header; + LPWSTR handle_file_path; + GUID event_guid; + [string] LPWSTR name; + DWORD data_size; + [unique, size_is(data_size)] BYTE *data; +} device_broadcast_handle; + +typedef union _device_broadcast switch (broadcast_device_type devicetype) event +{ + case BROADCAST_DEVICE_TYPE_DEVICEINTERFACE: device_broadcast_deviceinterface device_interface; + case BROADCAST_DEVICE_TYPE_HANDLE: device_broadcast_handle handle; +} device_broadcast; + [ uuid(57c680ac-7bce-4f39-97fd-ffea566754d5), endpoint("ncacn_np:[\\pipe\\wine_plugplay]"), -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6315

Vibhav Pant

4:45 p.m.

New subject: [PATCH v14 2/4] include/wine: Add header dbt.h.

From: Vibhav Pant <vibhavp(a)gmail.com> wine/dbt.h defines types used for registering and filter device notifications. --- include/Makefile.in | 1 + include/wine/dbt.h | 98 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 include/wine/dbt.h diff --git a/include/Makefile.in b/include/Makefile.in index bbf28cfc87e..c26b0ed18da 100644 --- a/include/Makefile.in +++ b/include/Makefile.in @@ -900,6 +900,7 @@ SOURCES = \ wine/asm.h \ wine/atsvc.idl \ wine/condrv.h \ + wine/dbt.h \ wine/dcetypes.idl \ wine/debug.h \ wine/dplaysp.h \ diff --git a/include/wine/dbt.h b/include/wine/dbt.h new file mode 100644 index 00000000000..d6955c1d9d7 --- /dev/null +++ b/include/wine/dbt.h @@ -0,0 +1,98 @@ +/* + * Definitions for registering device notifications + * + * Copyright 2024 Vibhav Pant + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#ifndef __WINE_WINE_DBT_H_ +#define __WINE_WINE_DBT_H_ + +#include <wine/debug.h> + +struct device_notification_details +{ + DWORD (CALLBACK *cb)(HANDLE handle, HDEVNOTIFY notify, DWORD flags, device_broadcast *data, HANDLE device_handle); + HANDLE handle; + DWORD devicetype; + union + { + struct + { + /* Used to implement DEVICE_NOTIFY_ALL_INTERFACE_CLASSES. If true, the class field below + * should be ignored. */ + BOOL all_classes; + GUID class; + } deviceinterface; + struct + { + /* The path for the device file the notification is originating from. */ + OBJECT_NAME_INFORMATION *name_info; + /* A handle to the device file. It is passed as is to the callback. */ + HANDLE device; + } device; + } filter; +}; + +extern HDEVNOTIFY WINAPI I_ScRegisterDeviceNotification( struct device_notification_details *details, + void *filter, DWORD flags ); +extern BOOL WINAPI I_ScUnregisterDeviceNotification( HDEVNOTIFY handle ); + +static inline const char *debugstr_device_broadcast(const device_broadcast *data) +{ + if (!data) return "(null)"; + + switch (data->devicetype) + { + case DBT_DEVTYP_DEVICEINTERFACE: + return wine_dbg_sprintf( "{%#lx DBT_DEVTYP_DEVICEINTERFACE %s %s}", + data->event.device_interface.header.reserved, + debugstr_guid( &data->event.device_interface.class_guid ), + debugstr_w( data->event.device_interface.name ) ); + case DBT_DEVTYP_HANDLE: + return wine_dbg_sprintf( "{%#lx DBT_DEVTYP_HANDLE %s %s %s %d %p}", + data->event.device_interface.header.reserved, + debugstr_w( data->event.handle.handle_file_path ), + debugstr_guid( &data->event.handle.event_guid ), + debugstr_w( data->event.handle.name ), + data->event.handle.data_size, data->event.handle.data ); + default: + return wine_dbg_sprintf( "{type=%#lx %p}\n", data->devicetype, data ); + } +} + +static inline const char * +debugstr_device_notification_details( const struct device_notification_details *details ) +{ + if (!details) return "(null)"; + switch (details->devicetype) + { + case DBT_DEVTYP_DEVICEINTERFACE: + if (details->filter.deviceinterface.all_classes) + return wine_dbg_sprintf( "{%p %p DBT_DEVTYP_DEVICEINTERFACE {all_classes=1}}", + details->cb, details->handle ); + return wine_dbg_sprintf( "{%p %p DBT_DEVTYP_DEVICEINTERFACE {class=%s}}", details->cb, + details->handle, + debugstr_guid( &details->filter.deviceinterface.class ) ); + case DBT_DEVTYP_HANDLE: + return wine_dbg_sprintf( "{%p %p DBT_DEVTYP_HANDLE {%p %p}}", details->cb, details->handle, + details->filter.device.name_info, details->filter.device.device ); + default: + return wine_dbg_sprintf( "{%p %p (unknown %#lx)}", details->cb, details->handle, details->devicetype); + } +} + +#endif /* __WINE_WINE_DBT_H_ */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6315

Vibhav Pant

4:45 p.m.

New subject: [PATCH v14 3/4] plugplay: Rewrite RPC interface to send and receive events using the newly introduced DEVICE_BROADCAST types.

From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/ntoskrnl.exe/pnp.c | 26 +++--- dlls/sechost/service.c | 79 ++++++++++-------- dlls/user32/Makefile.in | 1 + dlls/user32/input.c | 169 +++++++++++++++++++++++++++++--------- dlls/user32/plugplay.idl | 2 + include/wine/dbt.h | 4 +- include/wine/plugplay.idl | 4 +- programs/plugplay/main.c | 128 +++++++++++++++++++++++------ 8 files changed, 299 insertions(+), 114 deletions(-) create mode 100644 dlls/user32/plugplay.idl diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index 7444b81823c..8127c410941 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -695,11 +695,11 @@ static LONG WINAPI rpc_filter( EXCEPTION_POINTERS *eptr ) return I_RpcExceptionFilter( eptr->ExceptionRecord->ExceptionCode ); } -static void send_devicechange( DWORD code, void *data, unsigned int size ) +static void send_devicechange( DWORD code, device_broadcast *data ) { __TRY { - plugplay_send_event( code, data, size ); + plugplay_send_event( code, data ); } __EXCEPT(rpc_filter) { @@ -722,7 +722,7 @@ NTSTATUS WINAPI IoSetDeviceInterfaceState( UNICODE_STRING *name, BOOLEAN enable static const WCHAR hashW[] = {'#',0}; size_t namelen = name->Length / sizeof(WCHAR); - DEV_BROADCAST_DEVICEINTERFACE_W *broadcast; + device_broadcast broadcast = {0}; struct device_interface *iface; HANDLE iface_key, control_key; OBJECT_ATTRIBUTES attr = {0}; @@ -805,18 +805,16 @@ NTSTATUS WINAPI IoSetDeviceInterfaceState( UNICODE_STRING *name, BOOLEAN enable iface->enabled = enable; - len = offsetof(DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_name[namelen + 1]); - - if ((broadcast = heap_alloc( len ))) + broadcast.devicetype = DBT_DEVTYP_DEVICEINTERFACE; + broadcast.event.device_interface.name = heap_alloc( name->Length + 1 ); + if ( broadcast.event.device_interface.name ) { - broadcast->dbcc_size = len; - broadcast->dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE; - broadcast->dbcc_reserved = 0; - broadcast->dbcc_classguid = iface->interface_class; - lstrcpynW( broadcast->dbcc_name, name->Buffer, namelen + 1 ); - if (namelen > 1) broadcast->dbcc_name[1] = '\\'; - send_devicechange( enable ? DBT_DEVICEARRIVAL : DBT_DEVICEREMOVECOMPLETE, broadcast, len ); - heap_free( broadcast ); + broadcast.event.device_interface.class_guid = iface->interface_class; + lstrcpyW( broadcast.event.device_interface.name, name->Buffer ); + if ( namelen > 1 ) broadcast.event.device_interface.name[1] = '\\'; + send_devicechange( enable ? DBT_DEVICEARRIVAL : DBT_DEVICEREMOVECOMPLETE, + &broadcast ); + heap_free( broadcast.event.device_interface.name ); } return ret; } diff --git a/dlls/sechost/service.c b/dlls/sechost/service.c index 915b0b4ebe0..9de6383972a 100644 --- a/dlls/sechost/service.c +++ b/dlls/sechost/service.c @@ -36,6 +36,8 @@ #include "svcctl.h" #include "plugplay.h" +#include "wine/dbt.h" + WINE_DEFAULT_DEBUG_CHANNEL(service); struct notify_data @@ -1973,17 +1975,6 @@ BOOL WINAPI DECLSPEC_HOTPATCH StartServiceCtrlDispatcherW( const SERVICE_TABLE_E return service_run_main_thread(); } -struct device_notification_details -{ - DWORD (CALLBACK *cb)(HANDLE handle, DWORD flags, DEV_BROADCAST_HDR *header); - HANDLE handle; - union - { - DEV_BROADCAST_HDR header; - DEV_BROADCAST_DEVICEINTERFACE_W iface; - } filter; -}; - static HANDLE device_notify_thread; static struct list device_notify_list = LIST_INIT(device_notify_list); @@ -1993,23 +1984,27 @@ struct device_notify_registration struct device_notification_details details; }; -static BOOL notification_filter_matches( DEV_BROADCAST_HDR *filter, DEV_BROADCAST_HDR *event ) +static BOOL notification_filter_matches( struct device_notification_details *details, + device_broadcast *data ) { - if (!filter->dbch_devicetype) return TRUE; - if (filter->dbch_devicetype != event->dbch_devicetype) return FALSE; + if (!details->devicetype) return TRUE; + if (details->devicetype != data->devicetype) return FALSE; - if (filter->dbch_devicetype == DBT_DEVTYP_DEVICEINTERFACE) + if (details->devicetype == DBT_DEVTYP_DEVICEINTERFACE) { - DEV_BROADCAST_DEVICEINTERFACE_W *filter_iface = (DEV_BROADCAST_DEVICEINTERFACE_W *)filter; - DEV_BROADCAST_DEVICEINTERFACE_W *event_iface = (DEV_BROADCAST_DEVICEINTERFACE_W *)event; - if (filter_iface->dbcc_size == offsetof(DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_classguid)) return TRUE; - return IsEqualGUID( &filter_iface->dbcc_classguid, &event_iface->dbcc_classguid ); + if (details->filter.deviceinterface.all_classes) return TRUE; + return IsEqualGUID( &details->filter.deviceinterface.class, &data->event.device_interface.class_guid ); } - FIXME( "Filter dbch_devicetype %lu not implemented\n", filter->dbch_devicetype ); - return TRUE; + return !lstrcmpiW( details->filter.device.name_info->Name.Buffer, data->event.handle.handle_file_path ); } +struct device_notification_details_with_reg +{ + HDEVNOTIFY notify_handle; + struct device_notification_details details; +}; + static DWORD WINAPI device_notify_proc( void *arg ) { WCHAR endpoint[] = L"\\pipe\\wine_plugplay"; @@ -2017,12 +2012,11 @@ static DWORD WINAPI device_notify_proc( void *arg ) RPC_WSTR binding_str; DWORD err = ERROR_SUCCESS; struct device_notify_registration *registration; - struct device_notification_details *details_copy; + struct device_notification_details_with_reg *details_copy; unsigned int details_copy_nelems, details_copy_size; plugplay_rpc_handle handle = NULL; DWORD code = 0; - unsigned int i, size; - BYTE *buf; + unsigned int i; SetThreadDescription( GetCurrentThread(), L"wine_sechost_device_notify" ); @@ -2060,10 +2054,12 @@ static DWORD WINAPI device_notify_proc( void *arg ) for (;;) { - buf = NULL; + device_broadcast broadcast_data = { 0 }; + WCHAR *str; + __TRY { - code = plugplay_get_event( handle, &buf, &size ); + code = plugplay_get_event( handle, &broadcast_data ); err = ERROR_SUCCESS; } __EXCEPT(rpc_filter) @@ -2084,7 +2080,8 @@ static DWORD WINAPI device_notify_proc( void *arg ) EnterCriticalSection( &service_cs ); LIST_FOR_EACH_ENTRY(registration, &device_notify_list, struct device_notify_registration, entry) { - details_copy[i++] = registration->details; + details_copy[i].details = registration->details; + details_copy[i++].notify_handle = registration; details_copy_nelems++; if (i == details_copy_size) { @@ -2092,14 +2089,30 @@ static DWORD WINAPI device_notify_proc( void *arg ) details_copy = realloc( details_copy, details_copy_size * sizeof(*details_copy) ); } } - LeaveCriticalSection(&service_cs); + LeaveCriticalSection( &service_cs ); for (i = 0; i < details_copy_nelems; i++) { - if (!notification_filter_matches( &details_copy[i].filter.header, (DEV_BROADCAST_HDR *)buf )) continue; - details_copy[i].cb( details_copy[i].handle, code, (DEV_BROADCAST_HDR *)buf ); + struct device_notification_details *details = &details_copy[i].details; + HANDLE device = code == DBT_DEVTYP_HANDLE ? details->filter.device.device : NULL; + + if (!notification_filter_matches( details, &broadcast_data )) continue; + details->cb( details->handle, details_copy[i].notify_handle, code, &broadcast_data, device ); + } + switch (broadcast_data.devicetype) + { + case DBT_DEVTYP_DEVICEINTERFACE: + str = broadcast_data.event.device_interface.name; + break; + case DBT_DEVTYP_HANDLE: + str = broadcast_data.event.handle.name; + MIDL_user_free( broadcast_data.event.handle.handle_file_path ); + if (broadcast_data.event.handle.name) + MIDL_user_free( broadcast_data.event.handle.name ); + if (broadcast_data.event.handle.data_size != 0) + MIDL_user_free( broadcast_data.event.handle.data ); } - MIDL_user_free(buf); + if (str) MIDL_user_free( str ); } __TRY @@ -2124,7 +2137,7 @@ HDEVNOTIFY WINAPI I_ScRegisterDeviceNotification( struct device_notification_det { struct device_notify_registration *registration; - TRACE("callback %p, handle %p, filter %p, flags %#lx\n", details->cb, details->handle, filter, flags); + TRACE("details %s, filter %p, flags %#lx\n", debugstr_device_notification_details(details), filter, flags); if (!(registration = malloc( sizeof(struct device_notify_registration) ))) { @@ -2160,6 +2173,8 @@ BOOL WINAPI I_ScUnregisterDeviceNotification( HDEVNOTIFY handle ) EnterCriticalSection( &service_cs ); list_remove( ®istration->entry ); LeaveCriticalSection(&service_cs); + if (registration->details.devicetype == DBT_DEVTYP_HANDLE) + free( registration->details.filter.device.name_info ); free( registration ); return TRUE; } diff --git a/dlls/user32/Makefile.in b/dlls/user32/Makefile.in index 5f624559931..5b1c775d201 100644 --- a/dlls/user32/Makefile.in +++ b/dlls/user32/Makefile.in @@ -31,6 +31,7 @@ SOURCES = \ misc.c \ msgbox.c \ nonclient.c \ + plugplay.idl \ property.c \ resource.c \ resources/ocr_appstarting.svg \ diff --git a/dlls/user32/input.c b/dlls/user32/input.c index 108592f018f..68a113101a4 100644 --- a/dlls/user32/input.c +++ b/dlls/user32/input.c @@ -24,11 +24,16 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */ +#include "ntstatus.h" +#define WIN32_NO_STATUS #include "user_private.h" #include "dbt.h" #include "wine/server.h" #include "wine/debug.h" +#include "plugplay.h" +#include "wine/dbt.h" + WINE_DEFAULT_DEBUG_CHANNEL(win); WINE_DECLARE_DEBUG_CHANNEL(keyboard); @@ -497,74 +502,137 @@ BOOL WINAPI UnloadKeyboardLayout( HKL layout ) return FALSE; } - -static DWORD CALLBACK devnotify_window_callbackW(HANDLE handle, DWORD flags, DEV_BROADCAST_HDR *header) +static DWORD CALLBACK devnotify_window_callbackW( HANDLE handle, HDEVNOTIFY notify, DWORD flags, + device_broadcast *data, HANDLE device_handle ) { - SendMessageTimeoutW(handle, WM_DEVICECHANGE, flags, (LPARAM)header, SMTO_ABORTIFHUNG, 2000, NULL); + TRACE( "(%p, %p, %#lx, %s)\n", handle, notify, flags, debugstr_device_broadcast( data ) ); + + switch (data->devicetype) + { + case DBT_DEVTYP_DEVICEINTERFACE: + { + DEV_BROADCAST_DEVICEINTERFACE_W *iface; + size_t len = wcslen( data->event.device_interface.name ); + DWORD dbcc_size = offsetof( DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_name[len + 1] ); + + iface = malloc( dbcc_size ); + if (!iface) + return 0; + wcscpy( iface->dbcc_name, data->event.device_interface.name ); + iface->dbcc_size = dbcc_size; + iface->dbcc_devicetype = data->devicetype; + iface->dbcc_reserved = data->event.device_interface.header.reserved; + iface->dbcc_classguid = data->event.device_interface.class_guid; + SendMessageTimeoutW( handle, WM_DEVICECHANGE, flags, (LPARAM)iface, SMTO_ABORTIFHUNG, 2000, + NULL ); + free( iface ); + break; + } + case DBT_DEVTYP_HANDLE: + { + DEV_BROADCAST_HANDLE *bc_handle; + DWORD data_size = data->event.handle.data_size; + DWORD name_size = data->event.handle.name ? wcslen( data->event.handle.name ) + 1 : 0; + DWORD dbch_size = offsetof( DEV_BROADCAST_HANDLE, + dbch_data[name_size + data_size] ); + bc_handle = malloc( dbch_size ); + if (!bc_handle) + return 0; + bc_handle->dbch_size = dbch_size; + bc_handle->dbch_devicetype = DBT_DEVTYP_HANDLE; + bc_handle->dbch_reserved = data->event.handle.header.reserved; + bc_handle->dbch_handle = device_handle; + bc_handle->dbch_hdevnotify = notify; + bc_handle->dbch_eventguid = data->event.handle.event_guid; + bc_handle->dbch_nameoffset = name_size ? data_size : -1; + memcpy( bc_handle->dbch_data, data->event.handle.data, data_size ); + memcpy( &bc_handle->dbch_data[data_size], data->event.handle.name, name_size ); + SendMessageTimeoutW( handle, WM_DEVICECHANGE, flags, (LPARAM)bc_handle, SMTO_ABORTIFHUNG, + 2000, NULL ); + free( bc_handle ); + break; + } + default: + FIXME("unimplemented devicetype %#x\n", data->devicetype); + break; + } + return 0; } -static DWORD CALLBACK devnotify_window_callbackA(HANDLE handle, DWORD flags, DEV_BROADCAST_HDR *header) +static DWORD CALLBACK devnotify_window_callbackA( HANDLE handle, HDEVNOTIFY notify, DWORD flags, + device_broadcast *data, HANDLE device_handle ) { + TRACE("(%p, %p, %#lx, %s)\n", handle, notify, flags, debugstr_device_broadcast( data )); + if (flags & 0x8000) { - switch (header->dbch_devicetype) + switch (data->devicetype) { case DBT_DEVTYP_DEVICEINTERFACE: { - const DEV_BROADCAST_DEVICEINTERFACE_W *ifaceW = (const DEV_BROADCAST_DEVICEINTERFACE_W *)header; - size_t lenW = wcslen( ifaceW->dbcc_name ); + size_t lenW = wcslen( data->event.device_interface.name ); DEV_BROADCAST_DEVICEINTERFACE_A *ifaceA; DWORD lenA; if (!(ifaceA = malloc( offsetof(DEV_BROADCAST_DEVICEINTERFACE_A, dbcc_name[lenW * 3 + 1]) ))) return 0; - lenA = WideCharToMultiByte( CP_ACP, 0, ifaceW->dbcc_name, lenW + 1, + lenA = WideCharToMultiByte( CP_ACP, 0, data->event.device_interface.name, lenW + 1, ifaceA->dbcc_name, lenW * 3 + 1, NULL, NULL ); ifaceA->dbcc_size = offsetof(DEV_BROADCAST_DEVICEINTERFACE_A, dbcc_name[lenA + 1]); - ifaceA->dbcc_devicetype = ifaceW->dbcc_devicetype; - ifaceA->dbcc_reserved = ifaceW->dbcc_reserved; - ifaceA->dbcc_classguid = ifaceW->dbcc_classguid; + ifaceA->dbcc_devicetype = data->devicetype; + ifaceA->dbcc_reserved = data->event.device_interface.header.reserved; + ifaceA->dbcc_classguid = data->event.device_interface.class_guid; SendMessageTimeoutA( handle, WM_DEVICECHANGE, flags, (LPARAM)ifaceA, SMTO_ABORTIFHUNG, 2000, NULL ); free( ifaceA ); - return 0; + break; } - default: - FIXME( "unimplemented W to A mapping for %#lx\n", header->dbch_devicetype ); - /* fall through */ case DBT_DEVTYP_HANDLE: - case DBT_DEVTYP_OEM: + { + DEV_BROADCAST_HANDLE *bc_handle; + size_t lenW = data->event.handle.name ? wcslen( data->event.handle.name ) + 1: 0; + DWORD data_size = data->event.handle.data_size; + DWORD lenA = 0; + + bc_handle = malloc( offsetof( DEV_BROADCAST_HANDLE, dbch_data[lenW * 3 + 1 + data_size] ) ); + if (!bc_handle) + return 0; + + if (lenW) + lenA = WideCharToMultiByte( CP_ACP, 0, data->event.handle.name, lenW + 1, + (char *)&bc_handle->dbch_data[data_size], lenW * 3 + 1, + NULL, NULL ); + bc_handle->dbch_nameoffset = lenW ? data_size : -1; + bc_handle->dbch_size = offsetof( DEV_BROADCAST_HANDLE, dbch_data[lenA + 1] ); + bc_handle->dbch_devicetype = DBT_DEVTYP_HANDLE; + bc_handle->dbch_reserved = data->event.handle.header.reserved; + bc_handle->dbch_handle = device_handle; + bc_handle->dbch_hdevnotify = notify; + bc_handle->dbch_eventguid = data->event.handle.event_guid; + memcpy( bc_handle->dbch_data, data->event.handle.data, data_size ); + SendMessageTimeoutA( handle, WM_DEVICECHANGE, flags, (LPARAM)bc_handle, + SMTO_ABORTIFHUNG, 200, NULL ); + free( bc_handle ); + break; + } + default: + FIXME( "unimplemented W to A mapping for %#x\n", data->devicetype ); break; } } - SendMessageTimeoutA( handle, WM_DEVICECHANGE, flags, (LPARAM)header, SMTO_ABORTIFHUNG, 2000, NULL ); return 0; } -static DWORD CALLBACK devnotify_service_callback(HANDLE handle, DWORD flags, DEV_BROADCAST_HDR *header) +static DWORD CALLBACK devnotify_service_callback( HANDLE handle, HDEVNOTIFY notify, DWORD flags, + device_broadcast *data, HANDLE device_handle ) { FIXME("Support for service handles is not yet implemented!\n"); return 0; } -struct device_notification_details -{ - DWORD (CALLBACK *cb)(HANDLE handle, DWORD flags, DEV_BROADCAST_HDR *header); - HANDLE handle; - union - { - DEV_BROADCAST_HDR header; - DEV_BROADCAST_DEVICEINTERFACE_W iface; - } filter; -}; - -extern HDEVNOTIFY WINAPI I_ScRegisterDeviceNotification( struct device_notification_details *details, - void *filter, DWORD flags ); -extern BOOL WINAPI I_ScUnregisterDeviceNotification( HDEVNOTIFY handle ); - /*********************************************************************** * RegisterDeviceNotificationA (USER32.@) * @@ -580,7 +648,7 @@ HDEVNOTIFY WINAPI RegisterDeviceNotificationA( HANDLE handle, void *filter, DWOR */ HDEVNOTIFY WINAPI RegisterDeviceNotificationW( HANDLE handle, void *filter, DWORD flags ) { - struct device_notification_details details; + struct device_notification_details details = {0}; DEV_BROADCAST_HDR *header = filter; TRACE("handle %p, filter %p, flags %#lx\n", handle, filter, flags); @@ -597,21 +665,42 @@ HDEVNOTIFY WINAPI RegisterDeviceNotificationW( HANDLE handle, void *filter, DWOR return NULL; } - if (!header) details.filter.header.dbch_devicetype = 0; + if (!header) details.devicetype = 0; else if (header->dbch_devicetype == DBT_DEVTYP_DEVICEINTERFACE) { DEV_BROADCAST_DEVICEINTERFACE_W *iface = (DEV_BROADCAST_DEVICEINTERFACE_W *)header; - details.filter.iface = *iface; + details.devicetype = DBT_DEVTYP_DEVICEINTERFACE; if (flags & DEVICE_NOTIFY_ALL_INTERFACE_CLASSES) - details.filter.iface.dbcc_size = offsetof( DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_classguid ); + details.filter.deviceinterface.all_classes = TRUE; else - details.filter.iface.dbcc_size = offsetof( DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_name ); + details.filter.deviceinterface.class = iface->dbcc_classguid; } else if (header->dbch_devicetype == DBT_DEVTYP_HANDLE) { - FIXME( "DBT_DEVTYP_HANDLE filter type not implemented\n" ); - details.filter.header.dbch_devicetype = 0; + OBJECT_NAME_INFORMATION *name_info; + DEV_BROADCAST_HANDLE *handle = (DEV_BROADCAST_HANDLE *)header; + HANDLE device = handle->dbch_handle; + NTSTATUS status; + DWORD size; + + details.devicetype = DBT_DEVTYP_HANDLE; + status = NtQueryObject( device, ObjectNameInformation, NULL, 0, &size ); + if (status != STATUS_INFO_LENGTH_MISMATCH) + { + SetLastError( RtlNtStatusToDosError( status ) ); + return NULL; + } + + name_info = malloc( size ); + if (!name_info) + { + SetLastError( ERROR_NOT_ENOUGH_MEMORY ); + return NULL; + } + + details.filter.device.device = device; + details.filter.device.name_info = name_info; } else { diff --git a/dlls/user32/plugplay.idl b/dlls/user32/plugplay.idl new file mode 100644 index 00000000000..6b60a7efa75 --- /dev/null +++ b/dlls/user32/plugplay.idl @@ -0,0 +1,2 @@ +#pragma makedep header +#include "wine/plugplay.idl" diff --git a/include/wine/dbt.h b/include/wine/dbt.h index d6955c1d9d7..7ae6c04a666 100644 --- a/include/wine/dbt.h +++ b/include/wine/dbt.h @@ -68,9 +68,9 @@ static inline const char *debugstr_device_broadcast(const device_broadcast *data debugstr_w( data->event.handle.handle_file_path ), debugstr_guid( &data->event.handle.event_guid ), debugstr_w( data->event.handle.name ), - data->event.handle.data_size, data->event.handle.data ); + (int)data->event.handle.data_size, data->event.handle.data ); default: - return wine_dbg_sprintf( "{type=%#lx %p}\n", data->devicetype, data ); + return wine_dbg_sprintf( "{type=%#x %p}\n", data->devicetype, data ); } } diff --git a/include/wine/plugplay.idl b/include/wine/plugplay.idl index d5aaebe388e..97740f51c6e 100644 --- a/include/wine/plugplay.idl +++ b/include/wine/plugplay.idl @@ -65,7 +65,7 @@ interface plugplay typedef [context_handle] void *plugplay_rpc_handle; plugplay_rpc_handle plugplay_register_listener(); - DWORD plugplay_get_event([in] plugplay_rpc_handle handle, [out, size_is(,*size)] BYTE **data, [out] unsigned int *size); void plugplay_unregister_listener([in] plugplay_rpc_handle handle); - void plugplay_send_event([in] DWORD event_code, [in, size_is(size)] const BYTE *data, [in] unsigned int size); + void plugplay_send_event([in] DWORD event_code, [in] const device_broadcast *broadcast); + DWORD plugplay_get_event([in] plugplay_rpc_handle handle, [out] device_broadcast *broadcast); } diff --git a/programs/plugplay/main.c b/programs/plugplay/main.c index 8426f2204a3..ea1978493f8 100644 --- a/programs/plugplay/main.c +++ b/programs/plugplay/main.c @@ -19,12 +19,15 @@ #define WIN32_LEAN_AND_MEAN #include <windows.h> +#include <winternl.h> #include <dbt.h> #include "winsvc.h" #include "wine/debug.h" #include "wine/list.h" #include "plugplay.h" +#include "wine/dbt.h" + WINE_DEFAULT_DEBUG_CHANNEL(plugplay); static WCHAR plugplayW[] = L"PlugPlay"; @@ -64,11 +67,9 @@ struct event { struct list entry; DWORD code; - BYTE *data; - unsigned int size; + device_broadcast data; }; - static void destroy_listener( struct listener *listener ) { struct event *event, *next; @@ -79,7 +80,18 @@ static void destroy_listener( struct listener *listener ) LIST_FOR_EACH_ENTRY_SAFE(event, next, &listener->events, struct event, entry) { - MIDL_user_free( event->data ); + switch (event->data.devicetype) + { + case DBT_DEVTYP_HANDLE: + MIDL_user_free( event->data.event.handle.name ); + MIDL_user_free( event->data.event.handle.handle_file_path ); + MIDL_user_free( event->data.event.handle.data ); + break; + case DBT_DEVTYP_DEVICEINTERFACE: + MIDL_user_free( event->data.event.device_interface.name ); + break; + } + list_remove( &event->entry ); free( event ); } @@ -108,7 +120,7 @@ plugplay_rpc_handle __cdecl plugplay_register_listener(void) return listener; } -DWORD __cdecl plugplay_get_event( plugplay_rpc_handle handle, BYTE **data, unsigned int *size ) +DWORD __cdecl plugplay_get_event( plugplay_rpc_handle handle, device_broadcast *data ) { struct listener *listener = handle; struct event *event; @@ -117,17 +129,16 @@ DWORD __cdecl plugplay_get_event( plugplay_rpc_handle handle, BYTE **data, unsig EnterCriticalSection( &plugplay_cs ); - while (!(entry = list_head( &listener->events ))) + while(!(entry = list_head( &listener->events ))) SleepConditionVariableCS( &listener->cv, &plugplay_cs, INFINITE ); - event = LIST_ENTRY(entry, struct event, entry); - list_remove( &event->entry ); + event = LIST_ENTRY( entry, struct event, entry ); + list_remove(&event->entry); LeaveCriticalSection( &plugplay_cs ); ret = event->code; *data = event->data; - *size = event->size; free( event ); return ret; } @@ -137,35 +148,104 @@ void __cdecl plugplay_unregister_listener( plugplay_rpc_handle handle ) destroy_listener( handle ); } -void __cdecl plugplay_send_event( DWORD code, const BYTE *data, unsigned int size ) +void __cdecl plugplay_send_event( DWORD event_code, const device_broadcast *data ) { struct listener *listener; - struct event *event; + const WCHAR *name_src = NULL; + DWORD str_size = 0; - BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, code, (LPARAM)data ); - BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, DBT_DEVNODES_CHANGED, 0 ); + TRACE( "(%#lx, %s)\n", event_code, debugstr_device_broadcast( data ) ); - EnterCriticalSection( &plugplay_cs ); + switch(data->devicetype) + { + case DBT_DEVTYP_HANDLE: + name_src = data->event.handle.name; + break; + case DBT_DEVTYP_DEVICEINTERFACE: + { + const device_broadcast_deviceinterface *event = &data->event.device_interface; + DWORD name_len = wcslen( event->name ) + 1; + DWORD dbcc_size = offsetof( DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_name[name_len] ); + DEV_BROADCAST_DEVICEINTERFACE_W *iface; + + name_src = event->name; + + iface = malloc( dbcc_size ); + if (!iface) break; + iface->dbcc_size = dbcc_size; + iface->dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE; + iface->dbcc_reserved = event->header.reserved; + iface->dbcc_classguid = event->class_guid; + memcpy( iface->dbcc_name, event->name, name_len ); + BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, event_code, (LPARAM)iface ); + BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, DBT_DEVNODES_CHANGED, 0 ); + free( iface ); + break; + } + default: + ERR("unknown devicetype value: %#x\n", data->devicetype); + return; + } + str_size = name_src ? (lstrlenW(name_src) + 1) * sizeof(WCHAR) : 0; + + EnterCriticalSection( &plugplay_cs ); LIST_FOR_EACH_ENTRY(listener, &listener_list, struct listener, entry) { - if (!(event = malloc( sizeof(*event) ))) - break; + struct event *event = calloc( 1, sizeof(*event)); + if (!event) break; - if (!(event->data = malloc( size ))) + event->code = event_code; + + event->data = *data; + if (data->devicetype == DBT_DEVTYP_HANDLE) { - free( event ); - break; + DWORD size = sizeof(WCHAR) * ( lstrlenW( data->event.handle.handle_file_path ) + 1 ); + event->data.event.handle.handle_file_path = malloc( size ); + if (!event->data.event.handle.handle_file_path) + { + free( event ); + break; + } + memcpy( event->data.event.handle.handle_file_path, data->event.handle.handle_file_path, size ); + + if (data->event.handle.data_size != 0) + { + event->data.event.handle.data = malloc( data->event.handle.data_size ); + if (!event->data.event.handle.data) + { + free( event->data.event.handle.handle_file_path ); + free( event ); + break; + } + memcpy( event->data.event.handle.data, data->event.handle.data, data->event.handle.data_size); + } + } + if (str_size != 0) + { + WCHAR *str = malloc( str_size ); + if (!str) + { + if (data->devicetype == DBT_DEVTYP_HANDLE) + { + free( event->data.event.handle.handle_file_path ); + free( event->data.event.handle.data ); + } + free( event ); + break; + } + memcpy( str, name_src, str_size ); + + if (data->devicetype == DBT_DEVTYP_HANDLE) + event->data.event.handle.name = str; + else + event->data.event.device_interface.name = str; } - - event->code = code; - memcpy( event->data, data, size ); - event->size = size; list_add_tail( &listener->events, &event->entry ); WakeConditionVariable( &listener->cv ); } - LeaveCriticalSection( &plugplay_cs ); + return; } static DWORD WINAPI service_handler( DWORD ctrl, DWORD event_type, LPVOID event_data, LPVOID context ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6315

Vibhav Pant

4:46 p.m.

New subject: [PATCH v14 4/4] plugplay: Broadcast PnP events on a dedicated thread.

From: Vibhav Pant <vibhavp(a)gmail.com> As incoming events in plugplay_send_event are additionally broadcast asynchronously, free-ing them immediately after the call to BroadcastSystemMessageW would result in receiving threads potentially accessing invalid memory. To prevent this, perform these broadcasts synchronously on a dedicated thread with its own listener and event queue, which is spawned during service startup. For events of type DBT_DEVTYP_DEVICEINTERFACE, plugplay_send_event will stick an extra copy of the event into the listener's event queue, which then gets broadcast synchronously (and then freed) in the "broadcast listener" thread. --- programs/plugplay/main.c | 250 ++++++++++++++++++++++++++------------- 1 file changed, 166 insertions(+), 84 deletions(-) diff --git a/programs/plugplay/main.c b/programs/plugplay/main.c index ea1978493f8..e70e9b91c83 100644 --- a/programs/plugplay/main.c +++ b/programs/plugplay/main.c @@ -1,5 +1,6 @@ /* * Copyright 2011 Hans Leidekker for CodeWeavers + * Copyright 2024 Vibhav Pant * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -63,6 +64,12 @@ struct listener CONDITION_VARIABLE cv; }; +/* A separate listener used to broadcast DBT_DEVTYP_DEVICEINTERFACE events received + * in plugplay_send_event. */ +static struct listener broadcast_listener; +#define EVENT_CODE_SHUTDOWN (~0) +static HANDLE broadcast_listener_thread; + struct event { struct list entry; @@ -70,15 +77,11 @@ struct event device_broadcast data; }; -static void destroy_listener( struct listener *listener ) +static void listener_free_events( struct list *events ) { struct event *event, *next; - EnterCriticalSection( &plugplay_cs ); - list_remove( &listener->entry ); - LeaveCriticalSection( &plugplay_cs ); - - LIST_FOR_EACH_ENTRY_SAFE(event, next, &listener->events, struct event, entry) + LIST_FOR_EACH_ENTRY_SAFE(event, next, events, struct event, entry) { switch (event->data.devicetype) { @@ -95,6 +98,59 @@ static void destroy_listener( struct listener *listener ) list_remove( &event->entry ); free( event ); } +} + +static DWORD WINAPI broadcast_listener_proc( void *arg ) +{ + while (TRUE) + { + device_broadcast broadcast = {0}; + const device_broadcast_deviceinterface *event; + DEV_BROADCAST_DEVICEINTERFACE_W *iface; + DWORD name_len; + DWORD dbcc_size; + DWORD code; + + code = plugplay_get_event( &broadcast_listener, &broadcast ); + if (code == EVENT_CODE_SHUTDOWN) break; + + event = &broadcast.event.device_interface; + name_len = wcslen( event->name ) + 1; + dbcc_size = offsetof( DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_name[name_len] ); + + iface = malloc( dbcc_size ); + if (!iface) continue; + + iface->dbcc_size = dbcc_size; + iface->dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE; + iface->dbcc_reserved = event->header.reserved; + iface->dbcc_classguid = event->class_guid; + memcpy( iface->dbcc_name, event->name, name_len ); + TRACE( "sending %s\n", debugstr_device_broadcast( &broadcast ) ); + BroadcastSystemMessageW( 0, NULL, BSF_FORCEIFHUNG | BSF_QUERY, code, (LPARAM)iface ); + BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, DBT_DEVNODES_CHANGED, 0 ); + free( iface ); + free( event->name ); + } + + EnterCriticalSection(&plugplay_cs); + /* Since broacast_listener is accessed outside of listener_list (and therefore can still be + * accessed by plugplay_send_event), we need to hold plugplay_cs while freeing any remaining + * events. */ + listener_free_events( &broadcast_listener.events ); + LeaveCriticalSection( &plugplay_cs ); + + TRACE( "shutting down the broadcast listener thread\n" ); + return 0; +} + +static void destroy_listener( struct listener *listener ) +{ + EnterCriticalSection( &plugplay_cs ); + list_remove( &listener->entry ); + LeaveCriticalSection( &plugplay_cs ); + + listener_free_events( &listener->events ); free( listener ); } @@ -103,15 +159,22 @@ void __RPC_USER plugplay_rpc_handle_rundown( plugplay_rpc_handle handle ) destroy_listener( handle ); } +static plugplay_rpc_handle plugplay_init_listener( struct listener *listener ) +{ + list_init( &listener->events ); + InitializeConditionVariable( &listener->cv ); + + return listener; +} + plugplay_rpc_handle __cdecl plugplay_register_listener(void) { struct listener *listener; - if (!(listener = calloc( 1, sizeof(*listener) ))) + if (!(listener = calloc( 1, sizeof(*listener)))) return NULL; - list_init( &listener->events ); - InitializeConditionVariable( &listener->cv ); + plugplay_init_listener( listener ); EnterCriticalSection( &plugplay_cs ); list_add_tail( &listener_list, &listener->entry ); @@ -148,99 +211,99 @@ void __cdecl plugplay_unregister_listener( plugplay_rpc_handle handle ) destroy_listener( handle ); } -void __cdecl plugplay_send_event( DWORD event_code, const device_broadcast *data ) +static struct event *new_event( DWORD event_code, const device_broadcast *src ) { - struct listener *listener; const WCHAR *name_src = NULL; - DWORD str_size = 0; + WCHAR *name_copy = NULL; + struct event *event; - TRACE( "(%#lx, %s)\n", event_code, debugstr_device_broadcast( data ) ); + event = calloc( 1, sizeof( *event ) ); + if (!event) return NULL; - switch(data->devicetype) + name_src = src->devicetype == DBT_DEVTYP_HANDLE ? src->event.handle.name + : src->event.device_interface.name; + if (name_src) { - case DBT_DEVTYP_HANDLE: - name_src = data->event.handle.name; - break; - case DBT_DEVTYP_DEVICEINTERFACE: - { - const device_broadcast_deviceinterface *event = &data->event.device_interface; - DWORD name_len = wcslen( event->name ) + 1; - DWORD dbcc_size = offsetof( DEV_BROADCAST_DEVICEINTERFACE_W, dbcc_name[name_len] ); - DEV_BROADCAST_DEVICEINTERFACE_W *iface; + SIZE_T str_size = (lstrlenW(name_src) + 1) * sizeof(WCHAR); + name_copy = malloc( str_size ); + if (!name_copy) + { + free( event ); + return NULL; + } + memcpy( name_copy, name_src, str_size ); + } - name_src = event->name; + event->code = event_code; + event->data = *src; + if (src->devicetype == DBT_DEVTYP_HANDLE) + { + const device_broadcast_handle *event_handle = &src->event.handle; + DWORD size = sizeof(WCHAR) * (lstrlenW( event_handle->handle_file_path ) + 1); + WCHAR *path = malloc( size ); + if (!path) + { + if (name_copy) free( name_copy ); + free( event ); + return NULL; + } + memcpy( path, event_handle->handle_file_path, size ); + event->data.event.handle.handle_file_path = path; - iface = malloc( dbcc_size ); - if (!iface) break; - iface->dbcc_size = dbcc_size; - iface->dbcc_devicetype = DBT_DEVTYP_DEVICEINTERFACE; - iface->dbcc_reserved = event->header.reserved; - iface->dbcc_classguid = event->class_guid; - memcpy( iface->dbcc_name, event->name, name_len ); - BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, event_code, (LPARAM)iface ); - BroadcastSystemMessageW( 0, NULL, WM_DEVICECHANGE, DBT_DEVNODES_CHANGED, 0 ); - free( iface ); - break; + if (event_handle->data_size != 0) + { + void *data = malloc( event_handle->data_size ); + if (!data) + { + if (name_copy) free( name_copy ); + free( path ); + free( event ); + return NULL; + } + memcpy( data, event_handle->data, event_handle->data_size ); + event->data.event.handle.data = data; + } + event->data.event.handle.name = name_copy; } - default: + else /* DBT_DEVTYP_DEVICEINTERFACE */ + { + event->data.event.device_interface.name = name_copy; + } + + return event; +} + +void __cdecl plugplay_send_event( DWORD event_code, const device_broadcast *data ) +{ + struct listener *listener; + + TRACE( "(%#lx, %s)\n", event_code, debugstr_device_broadcast( data ) ); + + if (data->devicetype != DBT_DEVTYP_HANDLE && data->devicetype != DBT_DEVTYP_DEVICEINTERFACE) + { ERR("unknown devicetype value: %#x\n", data->devicetype); return; } - str_size = name_src ? (lstrlenW(name_src) + 1) * sizeof(WCHAR) : 0; EnterCriticalSection( &plugplay_cs ); - LIST_FOR_EACH_ENTRY(listener, &listener_list, struct listener, entry) + /* For DBT_DEVTYP_DEVICEINTERFACE events, stick an extra copy of the event into + * broadcast_listener's queue. */ + if (data->devicetype == DBT_DEVTYP_DEVICEINTERFACE) { - struct event *event = calloc( 1, sizeof(*event)); - if (!event) break; - - event->code = event_code; - - event->data = *data; - if (data->devicetype == DBT_DEVTYP_HANDLE) + struct event *event = new_event( event_code, data ); + if (event) { - DWORD size = sizeof(WCHAR) * ( lstrlenW( data->event.handle.handle_file_path ) + 1 ); - event->data.event.handle.handle_file_path = malloc( size ); - if (!event->data.event.handle.handle_file_path) - { - free( event ); - break; - } - memcpy( event->data.event.handle.handle_file_path, data->event.handle.handle_file_path, size ); - - if (data->event.handle.data_size != 0) - { - event->data.event.handle.data = malloc( data->event.handle.data_size ); - if (!event->data.event.handle.data) - { - free( event->data.event.handle.handle_file_path ); - free( event ); - break; - } - memcpy( event->data.event.handle.data, data->event.handle.data, data->event.handle.data_size); - } + list_add_tail( &broadcast_listener.events, &event->entry ); + WakeConditionVariable( &broadcast_listener.cv ); } - if (str_size != 0) - { - WCHAR *str = malloc( str_size ); - if (!str) - { - if (data->devicetype == DBT_DEVTYP_HANDLE) - { - free( event->data.event.handle.handle_file_path ); - free( event->data.event.handle.data ); - } - free( event ); - break; - } - memcpy( str, name_src, str_size ); + } + + LIST_FOR_EACH_ENTRY(listener, &listener_list, struct listener, entry) + { + struct event *event = new_event( event_code, data ); + if (!event) break; - if (data->devicetype == DBT_DEVTYP_HANDLE) - event->data.event.handle.name = str; - else - event->data.event.device_interface.name = str; - } list_add_tail( &listener->events, &event->entry ); WakeConditionVariable( &listener->cv ); } @@ -283,6 +346,7 @@ static void WINAPI ServiceMain( DWORD argc, LPWSTR *argv ) unsigned char protseq[] = "ncacn_np"; SERVICE_STATUS status; RPC_STATUS err; + struct event *shutdown_event; WINE_TRACE( "starting service\n" ); @@ -301,9 +365,19 @@ static void WINAPI ServiceMain( DWORD argc, LPWSTR *argv ) ERR("RpcServerListen() failed, error %lu\n", err); return; } + if (!(shutdown_event = calloc(1, sizeof(*shutdown_event)))) + { + ERR("Failed to allocate memory for shutdown event\n"); + return; + } + shutdown_event->code = EVENT_CODE_SHUTDOWN; stop_event = CreateEventW( NULL, TRUE, FALSE, NULL ); + plugplay_init_listener(&broadcast_listener); + broadcast_listener_thread = + CreateThread( NULL, 0, broadcast_listener_proc, NULL, 0, NULL ); + service_handle = RegisterServiceCtrlHandlerExW( plugplayW, service_handler, NULL ); if (!service_handle) return; @@ -319,6 +393,14 @@ static void WINAPI ServiceMain( DWORD argc, LPWSTR *argv ) WaitForSingleObject( stop_event, INFINITE ); + /* First, shutdown our broadcast listener. */ + EnterCriticalSection( &plugplay_cs ); + list_add_tail( &broadcast_listener.events, &shutdown_event->entry ); + LeaveCriticalSection( &plugplay_cs ); + + WaitForSingleObject( broadcast_listener_thread, INFINITE ); + CloseHandle( broadcast_listener_thread ); + RpcMgmtStopServerListening( NULL ); RpcServerUnregisterIf( plugplay_v0_0_s_ifspec, NULL, TRUE ); RpcMgmtWaitServerListen(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6315

Marvin

5:37 p.m.

New subject: [PATCH v14 4/4] plugplay: Broadcast PnP events on a dedicated thread.

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=147872 Your paranoid android. === debian11b (64 bit WoW report) === ddraw: ddraw7.c:3822: Test failed: Expected (0,0)-(640,480), got (0,0)-(1024,768). ddraw7.c:3847: Test failed: Expected (0,0)-(640,480), got (0,0)-(1024,768).

[PATCH v14 0/4] MR6315: plugplay: Rework how PnP notifications are sent over RPC to support DBT_DEVTYP_HANDLE events.

Marvin

tags

participants (4)