From 59971b42eea4cc75a3a26d0c17dd263b058e6c97 Mon Sep 17 00:00:00 2001 From: Bryan Berns Date: Tue, 20 Mar 2018 12:49:09 -0400 Subject: [PATCH] Updated Signal Handler (#287) https://github.com/PowerShell/Win32-OpenSSH/issues/1096 https://github.com/PowerShell/Win32-OpenSSH/issues/191 - Updated wait_for_multiple_objects_enhanced() to handle a no-event request while alterable. - Simplified wait_for_any_event() to by taking advantage of no-event alterable request in wait_for_multiple_objects_enhanced(). - Updated wait_for_any_event() to use MAX_CHILDREN limit instead of MAXIMUM_WAIT_OBJECTS limit. - Removed unnecessary ZeroMemory() call. - Created distinct definition MAXIMUM_WAIT_OBJECTS_ENHANCED and modified functions to use it. - Upped w32_select() event limit. - Modified wait_for_multiple_objects_enhanced() to allow for 0 millisecond wait. --- contrib/win32/win32compat/signal.c | 63 ++++++++------------- contrib/win32/win32compat/signal_internal.h | 5 +- contrib/win32/win32compat/signal_wait.c | 34 ++++++----- contrib/win32/win32compat/w32fd.c | 2 +- 4 files changed, 48 insertions(+), 56 deletions(-) diff --git a/contrib/win32/win32compat/signal.c b/contrib/win32/win32compat/signal.c index cb6fef5ebe5f..0e4dcad87690 100644 --- a/contrib/win32/win32compat/signal.c +++ b/contrib/win32/win32compat/signal.c @@ -260,59 +260,42 @@ sw_process_pending_signals() int wait_for_any_event(HANDLE* events, int num_events, DWORD milli_seconds) { - HANDLE all_events[MAXIMUM_WAIT_OBJECTS]; - DWORD num_all_events; + HANDLE all_events[MAXIMUM_WAIT_OBJECTS_ENHANCED]; DWORD live_children = children.num_children - children.num_zombies; + DWORD num_all_events = num_events + live_children; errno_t r = 0; - num_all_events = num_events + live_children; - - if (num_all_events > MAXIMUM_WAIT_OBJECTS) { - debug3("wait() - ERROR max events reached"); + if (num_all_events > MAXIMUM_WAIT_OBJECTS_ENHANCED) { + debug3("wait_for_any_event() - ERROR max events reached"); errno = ENOTSUP; return -1; } - if ((r = memcpy_s(all_events, MAXIMUM_WAIT_OBJECTS * sizeof(HANDLE), children.handles, live_children * sizeof(HANDLE)) != 0) || - ( r = memcpy_s(all_events + live_children, (MAXIMUM_WAIT_OBJECTS - live_children) * sizeof(HANDLE), events, num_events * sizeof(HANDLE)) != 0)) { + if ((r = memcpy_s(all_events, MAXIMUM_WAIT_OBJECTS_ENHANCED * sizeof(HANDLE), children.handles, live_children * sizeof(HANDLE)) != 0) || + ( r = memcpy_s(all_events + live_children, (MAXIMUM_WAIT_OBJECTS_ENHANCED - live_children) * sizeof(HANDLE), events, num_events * sizeof(HANDLE)) != 0)) { debug3("memcpy_s failed with error: %d.", r); return -1; } debug5("wait() on %d events and %d children", num_events, live_children); - /* TODO - implement signal catching and handling */ - if (num_all_events) { - DWORD ret = wait_for_multiple_objects_enhanced(num_all_events, all_events, milli_seconds, TRUE); - if ((ret >= WAIT_OBJECT_0_ENHANCED) && (ret <= WAIT_OBJECT_0_ENHANCED + num_all_events - 1)) { - /* woken up by event signalled - * is this due to a child process going down - */ - if (live_children && ((ret - WAIT_OBJECT_0_ENHANCED) < live_children)) { - sigaddset(&pending_signals, W32_SIGCHLD); - sw_child_to_zombie(ret - WAIT_OBJECT_0_ENHANCED); - } - } else if (ret == WAIT_IO_COMPLETION_ENHANCED) { - /* APC processed due to IO or signal*/ - } else if (ret == WAIT_TIMEOUT_ENHANCED) { - /* timed out */ - return 0; - } else { /* some other error*/ - errno = EOTHER; - debug3("ERROR: unxpected wait end: %d", ret); - return -1; - } - } else { - DWORD ret = SleepEx(milli_seconds, TRUE); - if (ret == WAIT_IO_COMPLETION) { - /* APC processed due to IO or signal*/ - } else if (ret == 0) { - /* timed out */ - return 0; - } else { /* some other error */ - errno = EOTHER; - debug3("ERROR: unxpected SleepEx error: %d", ret); - return -1; + DWORD ret = wait_for_multiple_objects_enhanced(num_all_events, all_events, milli_seconds, TRUE); + if ((ret >= WAIT_OBJECT_0_ENHANCED) && (ret <= WAIT_OBJECT_0_ENHANCED + num_all_events - 1)) { + /* woken up by event signaled + * is this due to a child process going down + */ + if (live_children && ((ret - WAIT_OBJECT_0_ENHANCED) < live_children)) { + sigaddset(&pending_signals, W32_SIGCHLD); + sw_child_to_zombie(ret - WAIT_OBJECT_0_ENHANCED); } + } else if (ret == WAIT_IO_COMPLETION_ENHANCED) { + /* APC processed due to IO or signal*/ + } else if (ret == WAIT_TIMEOUT_ENHANCED) { + /* timed out */ + return 0; + } else { /* some other error*/ + errno = EOTHER; + debug3("ERROR: unexpected wait end: %d", ret); + return -1; } if (pending_signals) diff --git a/contrib/win32/win32compat/signal_internal.h b/contrib/win32/win32compat/signal_internal.h index 5beae3b9dad3..715151c80af9 100644 --- a/contrib/win32/win32compat/signal_internal.h +++ b/contrib/win32/win32compat/signal_internal.h @@ -1,12 +1,12 @@ #include /* child processes */ -#define MAX_CHILDREN 500 +#define MAX_CHILDREN 512 struct _children { /* * array of handles and process_ids. - * intial (num_children - num_zombies) are alive + * initial (num_children - num_zombies) are alive * rest are zombies */ HANDLE handles[MAX_CHILDREN]; @@ -32,6 +32,7 @@ struct _timer_info { }; int sw_init_timer(); +#define MAXIMUM_WAIT_OBJECTS_ENHANCED 1024 #define WAIT_OBJECT_0_ENHANCED 0x00000000 #define WAIT_ABANDONED_0_ENHANCED 0x10000000 #define WAIT_TIMEOUT_ENHANCED 0x20000000 diff --git a/contrib/win32/win32compat/signal_wait.c b/contrib/win32/win32compat/signal_wait.c index d2482d4af789..6a767c379db8 100644 --- a/contrib/win32/win32compat/signal_wait.c +++ b/contrib/win32/win32compat/signal_wait.c @@ -73,15 +73,34 @@ DWORD wait_for_multiple_objects_enhanced(_In_ DWORD nCount, _In_ const HANDLE *lpHandles, _In_ DWORD dwMilliseconds, _In_ BOOL bAlertable) { - /* number of separate bins / threads required to monitor execution */ const DWORD bin_size = MAXIMUM_WAIT_OBJECTS; const DWORD bins_total = (nCount - 1) / bin_size + 1; DWORD return_value = WAIT_FAILED_ENHANCED; HANDLE wait_event = NULL; - wait_for_multiple_objects_struct *wait_bins = NULL; + wait_for_multiple_objects_struct wait_bins[(MAXIMUM_WAIT_OBJECTS_ENHANCED - 1) / MAXIMUM_WAIT_OBJECTS + 1] = { 0 }; DWORD wait_ret; + + /* protect against too many events */ + if (nCount > MAXIMUM_WAIT_OBJECTS_ENHANCED) + { + return WAIT_FAILED_ENHANCED; + } + + /* in the event that no events are passed and alterable, just do a sleep and + * and wait for wakeup call. This differs from the WaitForMultipleObjectsEx + * call which would return an error if no events are passed to the function. */ + if (nCount == 0 && bAlertable) + { + DWORD wait_ret = SleepEx(dwMilliseconds, TRUE); + if (wait_ret == 0) + return WAIT_TIMEOUT_ENHANCED; + else if (wait_ret == WAIT_IO_COMPLETION) + return WAIT_IO_COMPLETION_ENHANCED; + else + return WAIT_FAILED_ENHANCED; + } /* if less than the normal maximum then just use the built-in function * to avoid the overhead of another thread */ @@ -108,21 +127,12 @@ wait_for_multiple_objects_enhanced(_In_ DWORD nCount, _In_ const HANDLE *lpHand return WAIT_FAILED_ENHANCED; } - /* setup synchronization event to flag when the main thread should wake up */ wait_event = CreateEvent(NULL, TRUE, FALSE, NULL); if (wait_event == NULL) { goto cleanup; } - /* allocate an area to communicate with our threads */ - wait_bins = (wait_for_multiple_objects_struct *) - calloc(bins_total, sizeof(wait_for_multiple_objects_struct)); - if (wait_bins == NULL) { - goto cleanup; - } - - ZeroMemory(wait_bins, bins_total * sizeof(wait_for_multiple_objects_struct)); /* initialize each thread that handles up to MAXIMUM_WAIT_OBJECTS each */ for (DWORD bin = 0; bin < bins_total; bin++) { @@ -220,7 +230,5 @@ wait_for_multiple_objects_enhanced(_In_ DWORD nCount, _In_ const HANDLE *lpHand if (wait_event) CloseHandle(wait_event); - if (wait_bins) - free(wait_bins); return return_value; } diff --git a/contrib/win32/win32compat/w32fd.c b/contrib/win32/win32compat/w32fd.c index 8fca70a4915b..65757d542d07 100644 --- a/contrib/win32/win32compat/w32fd.c +++ b/contrib/win32/win32compat/w32fd.c @@ -654,7 +654,7 @@ w32_fcntl(int fd, int cmd, ... /* arg */) return ret; } -#define SELECT_EVENT_LIMIT 32 +#define SELECT_EVENT_LIMIT 512 int w32_select(int fds, w32_fd_set* readfds, w32_fd_set* writefds, w32_fd_set* exceptfds, const struct timeval *timeout) {