Hi, I modified lib/select.c to fix misbehaving select() on windows when select() is simultaneously called from multiple threads.
- I used a stack to reuse events created by CreateEvent() in similar way to the original code. I have no idea why event handle was reused in original code, so I preserved this behavior. - There is an obvious issue that my fix depends on pthread which is probably unacceptable but I have no idea how to use pthread only when pthread is actually used. By the way the problem I fixed was that when multiple threads call select() simultaneously, select() invocation with the fd that received an event often remains blocked and some other select() invocation returns. Please let me know what you think of this patch. Regards, Akash Rawal
>From 06a3b6cf0f4f9a7599a5595ebde73c1425025a5c Mon Sep 17 00:00:00 2001 From: Akash Rawal <[email protected]> Date: Wed, 24 May 2017 09:08:34 +0530 Subject: [PATCH] * lib/select.c: Make select() multi-thread safe in windows --- lib/select.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 80 insertions(+), 12 deletions(-) diff --git a/lib/select.c b/lib/select.c index ac86c499d..7464e5461 100644 --- a/lib/select.c +++ b/lib/select.c @@ -35,6 +35,7 @@ #include <stdio.h> #include <conio.h> #include <time.h> +#include <pthread.h> /* Get the overridden 'struct timeval'. */ #include <sys/time.h> @@ -105,6 +106,15 @@ IsSocketHandle (HANDLE h) /* Compute output fd_sets for libc descriptor FD (whose Windows handle is H). */ +static pthread_once_t windows_poll_handle_once = PTHREAD_ONCE_INIT; +static PNtQueryInformationFile NtQueryInformationFile; +void +static windows_poll_handle_init() +{ + NtQueryInformationFile = (PNtQueryInformationFile) + GetProcAddress (GetModuleHandle ("ntdll.dll"), + "NtQueryInformationFile"); +} static int windows_poll_handle (HANDLE h, int fd, @@ -119,8 +129,6 @@ windows_poll_handle (HANDLE h, int fd, BOOL bRet; IO_STATUS_BLOCK iosb; FILE_PIPE_LOCAL_INFORMATION fpli; - static PNtQueryInformationFile NtQueryInformationFile; - static BOOL once_only; read = write = except = FALSE; switch (GetFileType (h)) @@ -131,13 +139,7 @@ windows_poll_handle (HANDLE h, int fd, break; case FILE_TYPE_PIPE: - if (!once_only) - { - NtQueryInformationFile = (PNtQueryInformationFile) - GetProcAddress (GetModuleHandle ("ntdll.dll"), - "NtQueryInformationFile"); - once_only = TRUE; - } + pthread_once(&windows_poll_handle_once, windows_poll_handle_init); if (PeekNamedPipe (h, NULL, 0, NULL, &avail, NULL) != 0) { @@ -240,13 +242,77 @@ windows_poll_handle (HANDLE h, int fd, return ret; } +static struct +{ + HANDLE *data; + size_t len; + size_t alloc_len; + pthread_mutex_t lock; +} event_pool = {NULL, 0, 8, PTHREAD_MUTEX_INITIALIZER}; + +static HANDLE +get_hevent() +{ + HANDLE res; + + pthread_mutex_lock(&event_pool.lock); + + if (event_pool.len) + { + event_pool.len--; + res = event_pool.data[event_pool.len]; + } + else + { + res = CreateEvent (NULL, FALSE, FALSE, NULL); + } + + pthread_mutex_unlock(&event_pool.lock); + + return res; +} + +static void +return_hevent(HANDLE v) +{ + pthread_mutex_lock(&event_pool.lock); + + event_pool.len++; + + if (! event_pool.data) + { + event_pool.data = malloc(sizeof(HANDLE) * event_pool.alloc_len); + if (! event_pool.data) + { + fprintf(stderr, "Failed to allocate memory\n"); + abort(); + } + } + + if (event_pool.len > event_pool.alloc_len) + { + event_pool.alloc_len *= 2; + event_pool.data = realloc(event_pool.data, + sizeof(HANDLE) * event_pool.alloc_len); + if (! event_pool.data) + { + fprintf(stderr, "Failed to allocate memory\n"); + abort(); + } + } + + event_pool.data[event_pool.len - 1] = v; + + pthread_mutex_unlock(&event_pool.lock); +} + int rpl_select (int nfds, fd_set *rfds, fd_set *wfds, fd_set *xfds, struct timeval *timeout) #undef timeval { static struct timeval tv0; - static HANDLE hEvent; + HANDLE hEvent; HANDLE h, handle_array[FD_SETSIZE + 2]; fd_set handle_rfds, handle_wfds, handle_xfds; struct bitset rbits, wbits, xbits; @@ -273,8 +339,7 @@ rpl_select (int nfds, fd_set *rfds, fd_set *wfds, fd_set *xfds, } } - if (!hEvent) - hEvent = CreateEvent (NULL, FALSE, FALSE, NULL); + hEvent = get_hevent(); handle_array[0] = hEvent; nhandles = 1; @@ -346,6 +411,7 @@ rpl_select (int nfds, fd_set *rfds, fd_set *wfds, fd_set *xfds, if (!h) { errno = EBADF; + return_hevent(hEvent); return -1; } @@ -528,6 +594,8 @@ restart: } } + return_hevent(hEvent); + return rc; } -- 2.13.0
