Commit 44e79432 authored by Zebediah Figura's avatar Zebediah Figura Committed by Alexandre Julliard

winedevice: Make driver (un)loading synchronous.

This essentially reverts 440482d2. 440482d2 was aimed towards making it possible to load multiple drivers asynchronously, as well as to allow reentrancy. Unfortunately, asynchronicity is incorrect, as demonstrated by bug 38836, and some trivial testing shows that the SCM database lock is held for the entirety of the driver entry and exit routines, and that StartService() and ControlService() block until they complete. 57268241 and dd2624a2 nullified the effects of 440482d2, making driver loading all but synchronous (with the exception of the added 30 second timeout, but this is actually incorrect: drivers can block indefinitely). This patch therefore does not change any behaviour, but rather removes the use of threadpools and "async" functions, essentially reverting back to the implementation prior to 440482d2. The incidental change to unload_driver() made by that patch (viz. never to unload a driver without a DriverUnload() routine) is kept. Signed-off-by: 's avatarZebediah Figura <z.figura12@gmail.com> Signed-off-by: 's avatarAlexandre Julliard <julliard@winehq.org>
parent 0a648b27
......@@ -45,20 +45,14 @@ extern NTSTATUS CDECL wine_ntoskrnl_main_loop( HANDLE stop_event );
static const WCHAR winedeviceW[] = {'w','i','n','e','d','e','v','i','c','e',0};
static SERVICE_STATUS_HANDLE service_handle;
static PTP_CLEANUP_GROUP cleanup_group;
static SC_HANDLE manager_handle;
static BOOL shutdown_in_progress;
static HANDLE stop_event;
#define EVENT_STARTED 0
#define EVENT_ERROR 1
struct wine_driver
{
struct wine_rb_entry entry;
SERVICE_STATUS_HANDLE handle;
HANDLE events[2];
DRIVER_OBJECT *driver_obj;
WCHAR name[1];
};
......@@ -71,15 +65,6 @@ static int wine_drivers_rb_compare( const void *key, const struct wine_rb_entry
static struct wine_rb_tree wine_drivers = { wine_drivers_rb_compare };
static CRITICAL_SECTION drivers_cs;
static CRITICAL_SECTION_DEBUG critsect_debug =
{
0, 0, &drivers_cs,
{ &critsect_debug.ProcessLocksList, &critsect_debug.ProcessLocksList },
0, 0, { (DWORD_PTR)(__FILE__ ": drivers_cs") }
};
static CRITICAL_SECTION drivers_cs = { &critsect_debug, -1, 0, 0, 0, 0 };
/* find the LDR_MODULE corresponding to the driver module */
static LDR_MODULE *find_ldr_module( HMODULE module )
{
......@@ -299,11 +284,21 @@ static void set_service_status( SERVICE_STATUS_HANDLE handle, DWORD state, DWORD
SetServiceStatus( handle, &status );
}
static void WINAPI async_unload_driver( PTP_CALLBACK_INSTANCE instance, void *context )
/* call the driver unload function */
static void unload_driver( struct wine_rb_entry *entry, void *context )
{
struct wine_driver *driver = context;
struct wine_driver *driver = WINE_RB_ENTRY_VALUE( entry, struct wine_driver, entry );
DRIVER_OBJECT *driver_obj = driver->driver_obj;
LDR_MODULE *ldr;
LDR_MODULE *ldr = driver_obj->DriverSection;
if (!driver_obj->DriverUnload)
{
TRACE( "driver %s does not support unloading\n", wine_dbgstr_w(driver->name) );
return;
}
TRACE( "stopping driver %s\n", wine_dbgstr_w(driver->name) );
set_service_status( driver->handle, SERVICE_STOP_PENDING, 0 );
TRACE_(relay)( "\1Call driver unload %p (obj=%p)\n", driver_obj->DriverUnload, driver_obj );
......@@ -311,7 +306,6 @@ static void WINAPI async_unload_driver( PTP_CALLBACK_INSTANCE instance, void *co
TRACE_(relay)( "\1Ret driver unload %p (obj=%p)\n", driver_obj->DriverUnload, driver_obj );
ldr = driver_obj->DriverSection;
FreeLibrary( ldr->BaseAddress );
IoDeleteDriver( driver_obj );
ObDereferenceObject( driver_obj );
......@@ -321,57 +315,44 @@ static void WINAPI async_unload_driver( PTP_CALLBACK_INSTANCE instance, void *co
HeapFree( GetProcessHeap(), 0, driver );
}
/* call the driver unload function */
static NTSTATUS unload_driver( struct wine_rb_entry *entry, BOOL destroy )
/* load a driver and notify services.exe about the status change */
static NTSTATUS create_driver( const WCHAR *driver_name )
{
TP_CALLBACK_ENVIRON environment;
struct wine_driver *driver = WINE_RB_ENTRY_VALUE( entry, struct wine_driver, entry );
DRIVER_OBJECT *driver_obj = driver->driver_obj;
static const WCHAR driverW[] = {'\\','D','r','i','v','e','r','\\',0};
struct wine_driver *driver;
UNICODE_STRING drv_name;
NTSTATUS status;
DWORD length;
WCHAR *str;
length = FIELD_OFFSET( struct wine_driver, name[strlenW(driver_name) + 1] );
if (!(driver = HeapAlloc( GetProcessHeap(), 0, length )))
return STATUS_NO_MEMORY;
strcpyW( driver->name, driver_name );
driver->driver_obj = NULL;
if (!driver_obj)
if (!(driver->handle = (void *)OpenServiceW( manager_handle, driver_name, SERVICE_SET_STATUS )))
{
TRACE( "driver %s has not finished loading yet\n", wine_dbgstr_w(driver->name) );
HeapFree( GetProcessHeap(), 0, driver );
return STATUS_UNSUCCESSFUL;
}
if (!driver_obj->DriverUnload)
if (wine_rb_put( &wine_drivers, driver_name, &driver->entry ))
{
TRACE( "driver %s does not support unloading\n", wine_dbgstr_w(driver->name) );
CloseServiceHandle( (void *)driver->handle );
HeapFree( GetProcessHeap(), 0, driver );
return STATUS_UNSUCCESSFUL;
}
TRACE( "stopping driver %s\n", wine_dbgstr_w(driver->name) );
set_service_status( driver->handle, SERVICE_STOP_PENDING, 0 );
if (destroy)
if (!(str = HeapAlloc( GetProcessHeap(), 0, sizeof(driverW) + strlenW(driver->name)*sizeof(WCHAR) )))
{
async_unload_driver( NULL, driver );
return STATUS_SUCCESS;
status = STATUS_NO_MEMORY;
goto error;
}
wine_rb_remove( &wine_drivers, &driver->entry );
memset( &environment, 0, sizeof(environment) );
environment.Version = 1;
environment.CleanupGroup = cleanup_group;
/* don't block the service control handler */
if (!TrySubmitThreadpoolCallback( async_unload_driver, driver, &environment ))
async_unload_driver( NULL, driver );
return STATUS_SUCCESS;
}
static void WINAPI async_create_driver( PTP_CALLBACK_INSTANCE instance, void *context )
{
static const WCHAR driverW[] = {'\\','D','r','i','v','e','r','\\',0};
struct wine_driver *driver = context;
DRIVER_OBJECT *driver_obj;
UNICODE_STRING drv_name;
NTSTATUS status;
WCHAR *str;
if (!(str = HeapAlloc( GetProcessHeap(), 0, sizeof(driverW) + strlenW(driver->name)*sizeof(WCHAR) )))
goto error;
TRACE( "starting driver %s\n", wine_dbgstr_w(driver_name) );
set_service_status( driver->handle, SERVICE_START_PENDING, 0 );
lstrcpyW( str, driverW);
lstrcatW( str, driver->name );
......@@ -386,7 +367,7 @@ static void WINAPI async_create_driver( PTP_CALLBACK_INSTANCE instance, void *co
}
status = ObReferenceObjectByName( &drv_name, OBJ_CASE_INSENSITIVE, NULL,
0, NULL, KernelMode, NULL, (void **)&driver_obj );
0, NULL, KernelMode, NULL, (void **)&driver->driver_obj );
RtlFreeUnicodeString( &drv_name );
if (status != STATUS_SUCCESS)
{
......@@ -394,111 +375,16 @@ static void WINAPI async_create_driver( PTP_CALLBACK_INSTANCE instance, void *co
goto error;
}
SetEvent(driver->events[EVENT_STARTED]);
EnterCriticalSection( &drivers_cs );
driver->driver_obj = driver_obj;
set_service_status( driver->handle, SERVICE_RUNNING,
SERVICE_ACCEPT_STOP | SERVICE_ACCEPT_SHUTDOWN );
LeaveCriticalSection( &drivers_cs );
return;
return STATUS_SUCCESS;
error:
SetEvent(driver->events[EVENT_ERROR]);
EnterCriticalSection( &drivers_cs );
wine_rb_remove( &wine_drivers, &driver->entry );
LeaveCriticalSection( &drivers_cs );
set_service_status( driver->handle, SERVICE_STOPPED, 0 );
CloseServiceHandle( (void *)driver->handle );
wine_rb_remove( &wine_drivers, &driver->entry );
HeapFree( GetProcessHeap(), 0, driver );
}
/* load a driver and notify services.exe about the status change */
static NTSTATUS create_driver( const WCHAR *driver_name )
{
TP_CALLBACK_ENVIRON environment;
struct wine_driver *driver;
DWORD length;
DWORD ret;
length = FIELD_OFFSET( struct wine_driver, name[strlenW(driver_name) + 1] );
if (!(driver = HeapAlloc( GetProcessHeap(), 0, length )))
return STATUS_NO_MEMORY;
strcpyW( driver->name, driver_name );
driver->driver_obj = NULL;
if (!(driver->handle = (void *)OpenServiceW( manager_handle, driver_name, SERVICE_SET_STATUS )))
{
HeapFree( GetProcessHeap(), 0, driver );
return STATUS_UNSUCCESSFUL;
}
if (wine_rb_put( &wine_drivers, driver_name, &driver->entry ))
{
CloseServiceHandle( (void *)driver->handle );
HeapFree( GetProcessHeap(), 0, driver );
return STATUS_UNSUCCESSFUL;
}
TRACE( "starting driver %s\n", wine_dbgstr_w(driver_name) );
set_service_status( driver->handle, SERVICE_START_PENDING, 0 );
memset( &environment, 0, sizeof(environment) );
environment.Version = 1;
environment.CleanupGroup = cleanup_group;
driver->events[EVENT_STARTED] = CreateEventW(NULL, TRUE, FALSE, NULL);
driver->events[EVENT_ERROR] = CreateEventW(NULL, TRUE, FALSE, NULL);
/* don't block the service control handler */
if (!TrySubmitThreadpoolCallback( async_create_driver, driver, &environment ))
async_create_driver( NULL, driver );
/* Windows wait 30 Seconds */
ret = WaitForMultipleObjects(2, driver->events, FALSE, 30000);
if(ret == WAIT_OBJECT_0 + EVENT_ERROR)
return STATUS_UNSUCCESSFUL;
else if(ret == WAIT_TIMEOUT)
return ERROR_SERVICE_REQUEST_TIMEOUT;
return STATUS_SUCCESS;
}
static void wine_drivers_rb_destroy( struct wine_rb_entry *entry, void *context )
{
if (unload_driver( entry, TRUE ) != STATUS_SUCCESS)
{
struct wine_driver *driver = WINE_RB_ENTRY_VALUE( entry, struct wine_driver, entry );
CloseHandle(driver->events[EVENT_STARTED]);
CloseHandle(driver->events[EVENT_ERROR]);
ObDereferenceObject( driver->driver_obj );
CloseServiceHandle( (void *)driver->handle );
HeapFree( GetProcessHeap(), 0, driver );
}
}
static void WINAPI async_shutdown_drivers( PTP_CALLBACK_INSTANCE instance, void *context )
{
CloseThreadpoolCleanupGroupMembers( cleanup_group, FALSE, NULL );
EnterCriticalSection( &drivers_cs );
wine_rb_destroy( &wine_drivers, wine_drivers_rb_destroy, NULL );
LeaveCriticalSection( &drivers_cs );
SetEvent( stop_event );
}
static void shutdown_drivers( void )
{
if (shutdown_in_progress) return;
/* don't block the service control handler */
if (!TrySubmitThreadpoolCallback( async_shutdown_drivers, NULL, NULL ))
async_shutdown_drivers( NULL, NULL );
shutdown_in_progress = TRUE;
return status;
}
static DWORD device_handler( DWORD ctrl, const WCHAR *driver_name )
......@@ -506,10 +392,6 @@ static DWORD device_handler( DWORD ctrl, const WCHAR *driver_name )
struct wine_rb_entry *entry;
DWORD result = NO_ERROR;
if (shutdown_in_progress)
return ERROR_SERVICE_CANNOT_ACCEPT_CTRL;
EnterCriticalSection( &drivers_cs );
entry = wine_rb_get( &wine_drivers, driver_name );
switch (ctrl)
......@@ -521,14 +403,13 @@ static DWORD device_handler( DWORD ctrl, const WCHAR *driver_name )
case SERVICE_CONTROL_STOP:
if (!entry) break;
result = RtlNtStatusToDosError(unload_driver( entry, FALSE ));
unload_driver( entry, NULL );
break;
default:
FIXME( "got driver ctrl %x for %s\n", ctrl, wine_dbgstr_w(driver_name) );
break;
}
LeaveCriticalSection( &drivers_cs );
return result;
}
......@@ -548,7 +429,7 @@ static DWORD WINAPI service_handler( DWORD ctrl, DWORD event_type, LPVOID event_
case SERVICE_CONTROL_SHUTDOWN:
TRACE( "shutting down %s\n", wine_dbgstr_w(service_group) );
set_service_status( service_handle, SERVICE_STOP_PENDING, 0 );
shutdown_drivers();
SetEvent( stop_event );
return NO_ERROR;
default:
FIXME( "got service ctrl %x for %s\n", ctrl, wine_dbgstr_w(service_group) );
......@@ -564,8 +445,6 @@ static void WINAPI ServiceMain( DWORD argc, LPWSTR *argv )
if (!(stop_event = CreateEventW( NULL, TRUE, FALSE, NULL )))
return;
if (!(cleanup_group = CreateThreadpoolCleanupGroup()))
return;
if (!(manager_handle = OpenSCManagerW( NULL, NULL, SC_MANAGER_CONNECT )))
return;
if (!(service_handle = RegisterServiceCtrlHandlerExW( winedeviceW, service_handler, (void *)service_group )))
......@@ -578,9 +457,9 @@ static void WINAPI ServiceMain( DWORD argc, LPWSTR *argv )
wine_ntoskrnl_main_loop( stop_event );
TRACE( "service group %s stopped\n", wine_dbgstr_w(service_group) );
wine_rb_destroy( &wine_drivers, unload_driver, NULL );
set_service_status( service_handle, SERVICE_STOPPED, 0 );
CloseServiceHandle( manager_handle );
CloseThreadpoolCleanupGroup( cleanup_group );
CloseHandle( stop_event );
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment