From 683ee2aca95ee22523b7f8ac2dd625e18879d85c Mon Sep 17 00:00:00 2001 From: Hiroshi Hatake Date: Fri, 1 Mar 2024 15:37:49 +0900 Subject: [PATCH 1/5] subscribe: Ensure to wait for signal event Signed-off-by: Hiroshi Hatake --- ext/winevt/winevt.c | 2 ++ ext/winevt/winevt_c.h | 1 + ext/winevt/winevt_subscribe.c | 13 ++++++++++++- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/ext/winevt/winevt.c b/ext/winevt/winevt.c index 9dd3df9..621b5d8 100644 --- a/ext/winevt/winevt.c +++ b/ext/winevt/winevt.c @@ -7,6 +7,7 @@ VALUE rb_cSubscribe; VALUE rb_eWinevtQueryError; VALUE rb_eChannelNotFoundError; VALUE rb_eRemoteHandlerError; +VALUE rb_eSubscribeHandlerError; static ID id_call; @@ -20,6 +21,7 @@ Init_winevt(void) rb_eWinevtQueryError = rb_define_class_under(rb_cQuery, "Error", rb_eStandardError); rb_eChannelNotFoundError = rb_define_class_under(rb_cEventLog, "ChannelNotFoundError", rb_eStandardError); rb_eRemoteHandlerError = rb_define_class_under(rb_cSubscribe, "RemoteHandlerError", rb_eRuntimeError); + rb_eSubscribeHandlerError = rb_define_class_under(rb_cSubscribe, "SubscribeHandlerError", rb_eRuntimeError); Init_winevt_channel(rb_cEventLog); Init_winevt_bookmark(rb_cEventLog); diff --git a/ext/winevt/winevt_c.h b/ext/winevt/winevt_c.h index 2c894bc..baafc5e 100644 --- a/ext/winevt/winevt_c.h +++ b/ext/winevt/winevt_c.h @@ -61,6 +61,7 @@ extern VALUE rb_cSubscribe; extern VALUE rb_eWinevtQueryError; extern VALUE rb_eChannelNotFoundError; extern VALUE rb_eRemoteHandlerError; +extern VALUE rb_eSubscribeHandlerError; extern VALUE rb_cLocale; extern VALUE rb_cSession; diff --git a/ext/winevt/winevt_subscribe.c b/ext/winevt/winevt_subscribe.c index 577b5f2..68eba49 100644 --- a/ext/winevt/winevt_subscribe.c +++ b/ext/winevt/winevt_subscribe.c @@ -64,6 +64,8 @@ close_handles(struct WinevtSubscribe* winevtSubscribe) } winevtSubscribe->count = 0; + ResetEvent(winevtSubscribe->signalEvent); + if (winevtSubscribe->remoteHandle) { EvtClose(winevtSubscribe->remoteHandle); winevtSubscribe->remoteHandle = NULL; @@ -174,7 +176,7 @@ rb_winevt_subscribe_subscribe(int argc, VALUE* argv, VALUE self) struct WinevtSession* winevtSession; struct WinevtSubscribe* winevtSubscribe; - hSignalEvent = CreateEvent(NULL, FALSE, FALSE, NULL); + hSignalEvent = CreateEvent(NULL, TRUE, TRUE, NULL); TypedData_Get_Struct( self, struct WinevtSubscribe, &rb_winevt_subscribe_type, winevtSubscribe); @@ -341,6 +343,8 @@ rb_winevt_subscribe_next(VALUE self) EVT_HANDLE hEvents[SUBSCRIBE_ARRAY_SIZE]; ULONG count = 0; DWORD status = ERROR_SUCCESS; + DWORD dwWait = 0; + struct WinevtSubscribe* winevtSubscribe; TypedData_Get_Struct( @@ -355,6 +359,13 @@ rb_winevt_subscribe_next(VALUE self) return Qfalse; } + dwWait = WaitForSingleObject(winevtSubscribe->signalEvent, INFINITE); + if (dwWait == WAIT_FAILED) { + raise_system_error(rb_eSubscribeHandlerError, GetLastError()); + } else if (dwWait != WAIT_OBJECT_0) { + return Qfalse; + } + if (!EvtNext(winevtSubscribe->subscription, SUBSCRIBE_ARRAY_SIZE, hEvents, From 43148f35374cd559a526233e3a865b5a84953839 Mon Sep 17 00:00:00 2001 From: Hiroshi Hatake Date: Fri, 1 Mar 2024 15:59:11 +0900 Subject: [PATCH 2/5] workflows: Adjust Ruby versions Signed-off-by: Hiroshi Hatake --- .github/workflows/linux.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index a695fec..fe68db4 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -8,7 +8,7 @@ jobs: strategy: fail-fast: false matrix: - ruby: [ '2.4', '2.5', '2.6', '2.7', '3.0' ] + ruby: [ '3.0', '3.1', '3.2' ] os: - ubuntu-latest name: Ruby ${{ matrix.ruby }} building fat gem testing on ${{ matrix.os }} From d8299fbfbd723c05f43fb0f84cbf9da255058757 Mon Sep 17 00:00:00 2001 From: Hiroshi Hatake Date: Fri, 1 Mar 2024 16:56:07 +0900 Subject: [PATCH 3/5] subscribe: Interpret signal events From this pull type of subscription, we need to add the following actions: 1. Create an EventObject for handling signal events 2. Reset signal when ERROR_NO_MORE_ITEMS from EvtNext This should ensure the windows eventlog collection correctly. Also, we shouldn't specify the INFINITE waiting on WaitForSingleObject because this function is inside of enumerator. So, it causes infinite blocking for this case. ref: https://learn.microsoft.com/en-us/windows/win32/wes/subscribing-to-events#pull-subscriptions Signed-off-by: Hiroshi Hatake --- ext/winevt/winevt_subscribe.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/winevt/winevt_subscribe.c b/ext/winevt/winevt_subscribe.c index 68eba49..8c58342 100644 --- a/ext/winevt/winevt_subscribe.c +++ b/ext/winevt/winevt_subscribe.c @@ -64,8 +64,6 @@ close_handles(struct WinevtSubscribe* winevtSubscribe) } winevtSubscribe->count = 0; - ResetEvent(winevtSubscribe->signalEvent); - if (winevtSubscribe->remoteHandle) { EvtClose(winevtSubscribe->remoteHandle); winevtSubscribe->remoteHandle = NULL; @@ -359,7 +357,7 @@ rb_winevt_subscribe_next(VALUE self) return Qfalse; } - dwWait = WaitForSingleObject(winevtSubscribe->signalEvent, INFINITE); + dwWait = WaitForSingleObject(winevtSubscribe->signalEvent, 0); if (dwWait == WAIT_FAILED) { raise_system_error(rb_eSubscribeHandlerError, GetLastError()); } else if (dwWait != WAIT_OBJECT_0) { @@ -379,6 +377,8 @@ rb_winevt_subscribe_next(VALUE self) if (ERROR_NO_MORE_ITEMS != status) { return Qfalse; } + + ResetEvent(winevtSubscribe->signalEvent); } if (status == ERROR_SUCCESS) { From f9f833d7a7c5927339f0317af514915a8b5442b9 Mon Sep 17 00:00:00 2001 From: Hiroshi Hatake Date: Tue, 5 Mar 2024 18:27:27 +0900 Subject: [PATCH 4/5] subscribe: Add comments to describe what WaitForSingleObject does Signed-off-by: Hiroshi Hatake --- ext/winevt/winevt_subscribe.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ext/winevt/winevt_subscribe.c b/ext/winevt/winevt_subscribe.c index 8c58342..8946582 100644 --- a/ext/winevt/winevt_subscribe.c +++ b/ext/winevt/winevt_subscribe.c @@ -357,6 +357,12 @@ rb_winevt_subscribe_next(VALUE self) return Qfalse; } + /* If a signalEvent notifies a failure, raise + * SubscribeHandlerError to detect stale subscription. + * Note that we don't need to wait explicitly here. + * Because this function is inside of each enumerator. + * So, WaitForSingleObject should return immediately and should be + * processed with the latter each loop if there is no more items. */ dwWait = WaitForSingleObject(winevtSubscribe->signalEvent, 0); if (dwWait == WAIT_FAILED) { raise_system_error(rb_eSubscribeHandlerError, GetLastError()); From af6efe95b3475a8dd2a7e4ddbe2ff5d273a1dead Mon Sep 17 00:00:00 2001 From: Hiroshi Hatake Date: Wed, 6 Mar 2024 10:23:33 +0900 Subject: [PATCH 5/5] subscribe: Revise comments for WaitForSingleObject Signed-off-by: Hiroshi Hatake --- ext/winevt/winevt_subscribe.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/ext/winevt/winevt_subscribe.c b/ext/winevt/winevt_subscribe.c index 8946582..b8846f2 100644 --- a/ext/winevt/winevt_subscribe.c +++ b/ext/winevt/winevt_subscribe.c @@ -357,12 +357,16 @@ rb_winevt_subscribe_next(VALUE self) return Qfalse; } - /* If a signalEvent notifies a failure, raise - * SubscribeHandlerError to detect stale subscription. + /* If a signalEvent notifies whether a state of processed event(s) + * is existing or not. + * For checking for a result of WaitForSingleObject, + * we need to raise SubscribeHandlerError exception when + * WAIT_FAILED is detected for further investigations. * Note that we don't need to wait explicitly here. * Because this function is inside of each enumerator. * So, WaitForSingleObject should return immediately and should be - * processed with the latter each loop if there is no more items. */ + * processed with the latter each loops if there is no more items. + * Just intended to check that there is no errors here. */ dwWait = WaitForSingleObject(winevtSubscribe->signalEvent, 0); if (dwWait == WAIT_FAILED) { raise_system_error(rb_eSubscribeHandlerError, GetLastError());