Skip to content

Fix device cleanup to run on device disable #16

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JohnAZoidberg merged 1 commit into from
Jan 18, 2026
Merged

Fix device cleanup to run on device disable #16

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions FrameworkArgb/Device.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ Return Value:
WdfFdoInitSetFilter(DeviceInit);

WDF_OBJECT_ATTRIBUTES_INIT_CONTEXT_TYPE(&deviceAttributes, DEVICE_CONTEXT);
deviceAttributes.EvtCleanupCallback = FrameworkArgbEvtDeviceCleanup;

status = WdfDeviceCreate(&DeviceInit, &deviceAttributes, &device);

Expand Down Expand Up @@ -735,3 +736,39 @@ Return Value:

return status;
}

VOID
FrameworkArgbEvtDeviceCleanup(
_In_ WDFOBJECT DeviceObject
)
/*++
Routine Description:

Free all the resources allocated for the device.
Called when the device is removed or disabled.

Arguments:

DeviceObject - handle to a WDF Device object.

Return Value:

VOID.

--*/
{
PDEVICE_CONTEXT DeviceContext;

TraceInformation("%!FUNC! Entry");

DeviceContext = GetDeviceContext(DeviceObject);

// Close handle to EC driver
if (DeviceContext && DeviceContext->CrosEcHandle && DeviceContext->CrosEcHandle != INVALID_HANDLE_VALUE) {
CloseHandle(DeviceContext->CrosEcHandle);
DeviceContext->CrosEcHandle = INVALID_HANDLE_VALUE;
TraceInformation("%!FUNC! Closed CrosEc Handle");
Comment on lines +768 to +770
Copy link

Copilot AI Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting the handle to INVALID_HANDLE_VALUE after closing is good practice to prevent double-close bugs, but this happens after the close operation. If CloseHandle() fails, the handle should not be set to INVALID_HANDLE_VALUE. Consider checking the return value of CloseHandle() and only setting to INVALID_HANDLE_VALUE on success.

Suggested change
CloseHandle(DeviceContext->CrosEcHandle);
DeviceContext->CrosEcHandle = INVALID_HANDLE_VALUE;
TraceInformation("%!FUNC! Closed CrosEc Handle");
BOOL closeResult = CloseHandle(DeviceContext->CrosEcHandle);
if (!closeResult) {
TraceError("%!FUNC! Failed to close CrosEc Handle, error: %lu", GetLastError());
} else {
DeviceContext->CrosEcHandle = INVALID_HANDLE_VALUE;
TraceInformation("%!FUNC! Closed CrosEc Handle");
}

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@JohnAZoidberg JohnAZoidberg Jan 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, I wonder in what cases CloseHandle fails and if in those cases we want to avoid seetting the handle to invalid.
I would guess that in those cases the handle is actually already invalid or corrupted, so it may actually be good to still set it to INVALID_HANDLE_VALUE.

}

TraceInformation("%!FUNC! Exit");
}
1 change: 1 addition & 0 deletions FrameworkArgb/Device.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ typedef UCHAR HID_REPORT_DESCRIPTOR, * PHID_REPORT_DESCRIPTOR;
DRIVER_INITIALIZE DriverEntry;
EVT_WDF_DRIVER_DEVICE_ADD EvtDeviceAdd;
EVT_WDF_TIMER EvtTimerFunc;
EVT_WDF_OBJECT_CONTEXT_CLEANUP FrameworkArgbEvtDeviceCleanup;

#define MAX_LAMPARRAY_LAMP_COUNT 256
//#define LAMPARRAY_WIDTH 80000 // 80mm
Expand Down
15 changes: 5 additions & 10 deletions FrameworkArgb/Driver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ FrameworkArgbEvtDriverContextCleanup(
Routine Description:

Free all the resources allocated in DriverEntry.
Note: Device-specific cleanup (like EC handle) is done in
FrameworkArgbEvtDeviceCleanup, not here.

Arguments:

Expand All @@ -156,19 +158,10 @@ Return Value:

--*/
{
PDEVICE_CONTEXT DeviceContext;
UNREFERENCED_PARAMETER(DriverObject);

TraceInformation("%!FUNC! Entry");

DeviceContext = GetDeviceContext(DriverObject);
// Close handle to EC driver
if (DeviceContext && DeviceContext->CrosEcHandle && DeviceContext->CrosEcHandle != INVALID_HANDLE_VALUE) {
CloseHandle(DeviceContext->CrosEcHandle);
DeviceContext->CrosEcHandle = INVALID_HANDLE_VALUE;
TraceError("%!FUNC! Failed to close CrosEc Handle");
}
TraceInformation("%!FUNC! Closed CrosEc Handle");

//
// Stop WPP Tracing
//
Expand All @@ -177,4 +170,6 @@ Return Value:
#else
WPP_CLEANUP(WdfDriverWdmGetDriverObject((WDFDRIVER)DriverObject));
#endif

TraceInformation("%!FUNC! Exit");
}
Loading