From d605e5ecb86f09e58d70ab3c77f3967fd1176993 Mon Sep 17 00:00:00 2001 From: Shizuo Fujita Date: Fri, 31 Jul 2026 17:10:38 +0900 Subject: [PATCH] Fix FD_SETSIZE mismatch silently dropping watchers on Windows On Windows, fd_set is dimensioned by whatever FD_SETSIZE is in effect when winsock2.h is first pulled in. ev.c includes ruby.h (ev.c:41) long before it defines FD_SETSIZE (ev.c:213), and on RubyInstaller ruby.h reaches winsock2.h with -DFD_SETSIZE=2048 already on the command line. The unconditional define that followed could only move the bound EV_WIN_FD_SET checks against; it could not resize the array that bound indexes. The result was a silent cap. EV_WIN_FD_SET does nothing once fd_count reaches FD_SETSIZE, so watchers past the 1024th were dropped without an error, a return value, or a crash, while fd_set had room for 2048. Guard the define with #ifndef so we take whatever dimension is already in effect, and add a compile-time assert in ev_select.c so the two can no longer disagree. This is not the heap overflow it was reported as: the bound (1024) was smaller than the array (2048), so nothing was written out of bounds here. That failure needs a Ruby whose CPPFLAGS leaves FD_SETSIZE at the winsock2.h default of 64. The assert fails the build in that case rather than letting it through. Note that the existing assert (fd < FD_SETSIZE) in select_modify cannot catch any of this: ruby/assert.h defines NDEBUG, so every libev assert is compiled out of extensions that include ruby.h. Verified on: ruby 3.3.12 (2026-07-16 revision 0581089df9) [x64-mingw-ucrt] RUBY_PLATFORM = x64-mingw-ucrt gcc 16.1.0 (MSYS2 ucrt64) RbConfig CPPFLAGS = -DFD_SETSIZE=2048 -D_WIN32_WINNT=0x0600 ... Before: ev.c:213:10: warning: 'FD_SETSIZE' redefined : note: this is the location of the previous definition measured inside the translation unit: FD_SETSIZE == 1024, sizeof(fd_set) == 16392 (2048 slots) 1100 concurrent connections: 1023 echoed, 77 silently never watched After: no redefinition warning measured inside the translation unit: FD_SETSIZE == 2048, 2048 slots 1100 concurrent connections: 1100 echoed, 0 missing The assert was also checked in the failing direction by temporarily raising ev.c's define to 4096, which fails the build as intended. rspec: 58 examples, 1 failure (detach_race_condition_spec.rb:35), identical to the pre-change baseline on this platform. Co-Authored-By: Claude Opus 5 --- ext/libev/ev.c | 8 +++++++- ext/libev/ev_select.c | 10 ++++++++++ libev_win_select.diff | 34 +++++++++++++++++++++++++--------- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/ext/libev/ev.c b/ext/libev/ev.c index 8759ee5..5c18fd6 100644 --- a/ext/libev/ev.c +++ b/ext/libev/ev.c @@ -210,7 +210,13 @@ #else # include # define WIN32_LEAN_AND_MEAN -# define FD_SETSIZE 1024 +/* ruby.h above already pulled in winsock2.h, so fd_set may be dimensioned + * already. Defining FD_SETSIZE unconditionally here would only move the bound + * used by EV_WIN_FD_SET, not the array it indexes. Take whatever is in effect + * and only supply a default when nothing has been decided yet. */ +# ifndef FD_SETSIZE +# define FD_SETSIZE 1024 +# endif # include # include # ifndef EV_SELECT_IS_WINSOCKET diff --git a/ext/libev/ev_select.c b/ext/libev/ev_select.c index 28cb1bf..eccff2b 100644 --- a/ext/libev/ev_select.c +++ b/ext/libev/ev_select.c @@ -107,6 +107,16 @@ if (__i == ((fd_set *)(set))->fd_count) {\ #define EV_WIN_FD_ZERO(set) (((fd_set *)(set))->fd_count=0) #define EV_WIN_FD_ISSET(fd, set) __WSAFDIsSet((SOCKET)(fd), (fd_set *)(set)) #define EV_WIN_FD_COUNT(set) (((fd_set *)(set))->fd_count) + +/* +fd_set is dimensioned by whatever FD_SETSIZE was in effect when winsock2.h was +first pulled in, but EV_WIN_FD_SET and select_modify bound-check against the +FD_SETSIZE visible here. The two are decided by separate paths, so if the bound +ever exceeds the declared array we would write past the end of the allocation in +select_init. Catch that at build time instead. +*/ +typedef char coolio_fd_setsize_matches_fd_set[ + (sizeof (((fd_set *)0)->fd_array) / sizeof (SOCKET) >= (size_t)FD_SETSIZE) ? 1 : -1]; /* ######################################## */ #else #define EV_WIN_FD_CLR FD_CLR diff --git a/libev_win_select.diff b/libev_win_select.diff index 229f171..659d8e3 100644 --- a/libev_win_select.diff +++ b/libev_win_select.diff @@ -1,20 +1,26 @@ diff --git a/ext/libev/ev.c b/ext/libev/ev.c -index dae87f1..d15f6bd 100644 +index a59efb2..5c18fd6 100644 --- a/ext/libev/ev.c +++ b/ext/libev/ev.c -@@ -207,6 +207,7 @@ +@@ -210,6 +210,13 @@ #else # include # define WIN32_LEAN_AND_MEAN -+# define FD_SETSIZE 1024 ++/* ruby.h above already pulled in winsock2.h, so fd_set may be dimensioned ++ * already. Defining FD_SETSIZE unconditionally here would only move the bound ++ * used by EV_WIN_FD_SET, not the array it indexes. Take whatever is in effect ++ * and only supply a default when nothing has been decided yet. */ ++# ifndef FD_SETSIZE ++# define FD_SETSIZE 1024 ++# endif # include # include # ifndef EV_SELECT_IS_WINSOCKET diff --git a/ext/libev/ev_select.c b/ext/libev/ev_select.c -index f38d6ca..7050778 100644 +index ed1fc7a..eccff2b 100644 --- a/ext/libev/ev_select.c +++ b/ext/libev/ev_select.c -@@ -67,6 +67,54 @@ +@@ -67,6 +67,64 @@ #include @@ -58,6 +64,16 @@ index f38d6ca..7050778 100644 +#define EV_WIN_FD_ZERO(set) (((fd_set *)(set))->fd_count=0) +#define EV_WIN_FD_ISSET(fd, set) __WSAFDIsSet((SOCKET)(fd), (fd_set *)(set)) +#define EV_WIN_FD_COUNT(set) (((fd_set *)(set))->fd_count) ++ ++/* ++fd_set is dimensioned by whatever FD_SETSIZE was in effect when winsock2.h was ++first pulled in, but EV_WIN_FD_SET and select_modify bound-check against the ++FD_SETSIZE visible here. The two are decided by separate paths, so if the bound ++ever exceeds the declared array we would write past the end of the allocation in ++select_init. Catch that at build time instead. ++*/ ++typedef char coolio_fd_setsize_matches_fd_set[ ++ (sizeof (((fd_set *)0)->fd_array) / sizeof (SOCKET) >= (size_t)FD_SETSIZE) ? 1 : -1]; +/* ######################################## */ +#else +#define EV_WIN_FD_CLR FD_CLR @@ -69,7 +85,7 @@ index f38d6ca..7050778 100644 static void select_modify (EV_P_ int fd, int oev, int nev) { -@@ -91,17 +139,17 @@ select_modify (EV_P_ int fd, int oev, int nev) +@@ -91,17 +149,17 @@ select_modify (EV_P_ int fd, int oev, int nev) if ((oev ^ nev) & EV_READ) #endif if (nev & EV_READ) @@ -91,7 +107,7 @@ index f38d6ca..7050778 100644 #else -@@ -197,8 +245,8 @@ select_poll (EV_P_ ev_tstamp timeout) +@@ -197,8 +255,8 @@ select_poll (EV_P_ ev_tstamp timeout) { if (timeout) { @@ -102,7 +118,7 @@ index f38d6ca..7050778 100644 } return; -@@ -230,10 +278,10 @@ select_poll (EV_P_ ev_tstamp timeout) +@@ -230,10 +288,10 @@ select_poll (EV_P_ ev_tstamp timeout) int handle = fd; #endif @@ -116,7 +132,7 @@ index f38d6ca..7050778 100644 #endif if (expect_true (events)) -@@ -279,9 +327,9 @@ select_init (EV_P_ int flags) +@@ -280,9 +338,9 @@ select_init (EV_P_ int flags) backend_poll = select_poll; #if EV_SELECT_USE_FD_SET