Backport mutexes for socket and connection lists on win32 #4212

2 files changed, 105 insertions, 40 deletions

diff --git a/test/socket/test_tcp.rb b/test/socket/test_tcp.rb
index 11325fdedb..7962cbe959 100644
--- a/test/socket/test_tcp.rb
+++ b/test/socket/test_tcp.rb
@@ -95,4 +95,29 @@ class TestSocket_TCPSocket < Test::Unit::TestCase
       assert_raise(IO::WaitReadable) { svr.accept_nonblock(exception: true) }
     }
   end
+
+  def test_accept_multithread
+    attempts_count       = 5
+    server_threads_count = 3
+    client_threads_count = 3
+
+    attempts_count.times do
+      server_threads = Array.new(server_threads_count) do
+        Thread.new do
+          TCPServer.open("localhost", 0) do |server|
+            accept_threads = Array.new(client_threads_count) do
+              Thread.new { server.accept.close }
+            end
+            client_threads = Array.new(client_threads_count) do
+              Thread.new { TCPSocket.open(server.addr[3], server.addr[1]) }
+            end
+            client_threads.each(&:join)
+            accept_threads.each(&:join)
+          end
+        end
+      end
+
+      server_threads.each(&:join)
+    end
+  end
 end if defined?(TCPSocket)
diff --git a/win32/win32.c b/win32/win32.c
index 247ac918d4..6356e75cb3 100644
--- a/win32/win32.c
+++ b/win32/win32.c
@@ -689,9 +689,14 @@ rtc_error_handler(int e, const char *src, int line, const char *exe, const char
 #endif
 
 static CRITICAL_SECTION select_mutex;
+
+static CRITICAL_SECTION socklist_mutex;
 static st_table *socklist = NULL;
+
+static CRITICAL_SECTION conlist_mutex;
 static st_table *conlist = NULL;
 #define conlist_disabled ((st_table *)-1)
+
 static char *uenvarea;
 
 /* License: Ruby's */
@@ -716,11 +721,13 @@ free_conlist(st_data_t key, st_data_t val, st_data_t arg)
 static void
 constat_delete(HANDLE h)
 {
+    EnterCriticalSection(&conlist_mutex);
     if (conlist && conlist != conlist_disabled) {
 	st_data_t key = (st_data_t)h, val;
 	st_delete(conlist, &key, &val);
 	xfree((struct constat *)val);
     }
+    LeaveCriticalSection(&conlist_mutex);
 }
 
 /* License: Ruby's */
@@ -729,6 +736,8 @@ exit_handler(void)
 {
     WSACleanup();
     DeleteCriticalSection(&select_mutex);
+    DeleteCriticalSection(&socklist_mutex);
+    DeleteCriticalSection(&conlist_mutex);
     if (uenvarea) {
 	free(uenvarea);
 	uenvarea = NULL;
@@ -739,15 +748,20 @@ exit_handler(void)
 static void
 vm_exit_handler(ruby_vm_t *vm)
 {
+    EnterCriticalSection(&socklist_mutex);
     if (socklist) {
 	st_free_table(socklist);
 	socklist = NULL;
     }
+    LeaveCriticalSection(&socklist_mutex);
+
+    EnterCriticalSection(&conlist_mutex);
     if (conlist && conlist != conlist_disabled) {
 	st_foreach(conlist, free_conlist, 0);
 	st_free_table(conlist);
 	conlist = NULL;
     }
+    LeaveCriticalSection(&conlist_mutex);
 }
 
 /* License: Ruby's */
@@ -780,6 +794,8 @@ StartSockets(void)
 	rb_fatal("could not find version 2 of winsock dll");
 
     InitializeCriticalSection(&select_mutex);
+    InitializeCriticalSection(&socklist_mutex);
+    InitializeCriticalSection(&conlist_mutex);
 
     atexit(exit_handler);
 }
@@ -792,11 +808,17 @@ StartSockets(void)
 static inline int
 socklist_insert(SOCKET sock, int flag)
 {
+    int ret;
+
+    EnterCriticalSection(&socklist_mutex);
     if (!socklist) {
 	socklist = st_init_numtable();
 	install_vm_exit_handler();
     }
-    return st_insert(socklist, (st_data_t)sock, (st_data_t)flag);
+    ret = st_insert(socklist, (st_data_t)sock, (st_data_t)flag);
+    LeaveCriticalSection(&socklist_mutex);
+
+    return ret;
 }
 
 /* License: Ruby's */
@@ -806,11 +828,15 @@ socklist_lookup(SOCKET sock, int *flagp)
     st_data_t data;
     int ret;
 
-    if (!socklist)
-	return 0;
-    ret = st_lookup(socklist, (st_data_t)sock, (st_data_t *)&data);
-    if (ret && flagp)
-	*flagp = (int)data;
+    EnterCriticalSection(&socklist_mutex);
+    if (socklist) {
+	ret = st_lookup(socklist, (st_data_t)sock, (st_data_t *)&data);
+	if (ret && flagp)
+	    *flagp = (int)data;
+    } else {
+	ret = 0;
+    }
+    LeaveCriticalSection(&socklist_mutex);
 
     return ret;
 }
@@ -823,17 +849,21 @@ socklist_delete(SOCKET *sockp, int *flagp)
     st_data_t data;
     int ret;
 
-    if (!socklist)
-	return 0;
-    key = (st_data_t)*sockp;
-    if (flagp)
-	data = (st_data_t)*flagp;
-    ret = st_delete(socklist, &key, &data);
-    if (ret) {
-	*sockp = (SOCKET)key;
+    EnterCriticalSection(&socklist_mutex);
+    if (socklist) {
+	key = (st_data_t)*sockp;
 	if (flagp)
-	    *flagp = (int)data;
+	    data = (st_data_t)*flagp;
+	ret = st_delete(socklist, &key, &data);
+	if (ret) {
+	    *sockp = (SOCKET)key;
+	    if (flagp)
+		*flagp = (int)data;
+	}
+    } else {
+	ret = 0;
     }
+    LeaveCriticalSection(&socklist_mutex);
 
     return ret;
 }
@@ -6530,32 +6560,36 @@ constat_handle(HANDLE h)
 {
     st_data_t data;
     struct constat *p;
+
+    EnterCriticalSection(&conlist_mutex);
     if (!conlist) {
 	if (console_emulator_p()) {
 	    conlist = conlist_disabled;
-	    return NULL;
-	}
-	conlist = st_init_numtable();
-	install_vm_exit_handler();
-    }
-    else if (conlist == conlist_disabled) {
-	return NULL;
-    }
-    if (st_lookup(conlist, (st_data_t)h, &data)) {
-	p = (struct constat *)data;
-    }
-    else {
-	CONSOLE_SCREEN_BUFFER_INFO csbi;
-	p = ALLOC(struct constat);
-	p->vt100.state = constat_init;
-	p->vt100.attr = FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED;
-	p->vt100.reverse = 0;
-	p->vt100.saved.X = p->vt100.saved.Y = 0;
-	if (GetConsoleScreenBufferInfo(h, &csbi)) {
-	    p->vt100.attr = csbi.wAttributes;
+	} else {
+	    conlist = st_init_numtable();
+	    install_vm_exit_handler();
+	}
+    }
+    if (conlist != conlist_disabled) {
+	if (st_lookup(conlist, (st_data_t)h, &data)) {
+	    p = (struct constat *)data;
+	} else {
+	    CONSOLE_SCREEN_BUFFER_INFO csbi;
+	    p = ALLOC(struct constat);
+	    p->vt100.state = constat_init;
+	    p->vt100.attr = FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED;
+	    p->vt100.reverse = 0;
+	    p->vt100.saved.X = p->vt100.saved.Y = 0;
+	    if (GetConsoleScreenBufferInfo(h, &csbi)) {
+		p->vt100.attr = csbi.wAttributes;
+	    }
+	    st_insert(conlist, (st_data_t)h, (st_data_t)p);
 	}
-	st_insert(conlist, (st_data_t)h, (st_data_t)p);
+    } else {
+	p = NULL;
     }
+    LeaveCriticalSection(&conlist_mutex);
+
     return p;
 }
 
@@ -6565,10 +6599,16 @@ constat_reset(HANDLE h)
 {
     st_data_t data;
     struct constat *p;
-    if (!conlist || conlist == conlist_disabled) return;
-    if (!st_lookup(conlist, (st_data_t)h, &data)) return;
-    p = (struct constat *)data;
-    p->vt100.state = constat_init;
+
+    EnterCriticalSection(&conlist_mutex);
+    if (
+	conlist && conlist != conlist_disabled &&
+	st_lookup(conlist, (st_data_t)h, &data)
+    ) {
+	p = (struct constat *)data;
+	p->vt100.state = constat_init;
+    }
+    LeaveCriticalSection(&conlist_mutex);
 }
 
 #define FOREGROUND_MASK (FOREGROUND_BLUE | FOREGROUND_GREEN | FOREGROUND_RED | FOREGROUND_INTENSITY)