Project

General

Profile

Bug #17394

TCPServer is not thread safe on win32

Added by puchuu (Andrew Aladjev) 3 months ago. Updated 11 days ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:101458]

Description

Hello, it looks like win32 version of TCPServer is not thread safe. I've extracted the following code from project:

require "socket"
require "parallel"

::Parallel.each(1..2, :in_threads => 2) do
  ::TCPServer.open 0 do |server|
    thread = ::Thread.new { server.accept.close }
    ::TCPSocket.open "localhost", server.addr[1]
    thread.join
  end
end

It returns:

in `accept': Bad file descriptor - not a socket file descriptor (Errno::EBADF)

You can fix this code by using :in_threads => 1. This is unique windows issue: GNU/Linux, FreeBSD and OSX works perfect. I think something is wrong in win32 folder of ruby source code.

Can you please assign this issue to the developer familiar with win32 folder? Thank you.


Files

Screenshot_20201215_020316.png (11.4 KB) Screenshot_20201215_020316.png puchuu (Andrew Aladjev), 12/14/2020 11:03 PM
4220.patch (7.68 KB) 4220.patch Backport mutexes for socket and connection lists on win32 puchuu (Andrew Aladjev), 02/23/2021 10:49 PM

Updated by puchuu (Andrew Aladjev) 3 months ago

PS This issue is not 100% reproducible, please try to run script several times, thank you.

Updated by puchuu (Andrew Aladjev) 3 months ago

I've added a screenshot from windows virtual machine on mingw64 env.

#3

Updated by puchuu (Andrew Aladjev) 16 days ago

The problem is socklist_insert function, you can just add the following code:

socklist_insert(r, 0);
fprintf(stderr, "fit %d\n", is_socket(TO_SOCKET(fd)));

is_socket has a chance to be false. is_socket is just an alias to socklist_lookup. So we can see that socklist_insert and socklist_lookup somehow used in wrong way.

#4

Updated by puchuu (Andrew Aladjev) 16 days ago

You can easily reproduce this issue by using the following code:

// socklist_insert(r, 0);
st_data_t data;

int insert_result_0 = st_insert(socklist, (st_data_t)r, (st_data_t)0);
int lookup_result_0 = st_lookup(socklist, (st_data_t)r, &data);
fprintf(stderr, "fit 0 %d %d\n", insert_result_0, lookup_result_0);

int insert_result_1 = st_insert(socklist, (st_data_t)r, (st_data_t)0);
int lookup_result_1 = st_lookup(socklist, (st_data_t)r, &data);
fprintf(stderr, "fit 1 %d %d\n", insert_result_1, lookup_result_1);

It is possible to receive "fit 0 0 0" and "fit 1 1 1" on windows, it means that first st_insert silently failed without error code, but second attempt succeed.

#5

Updated by puchuu (Andrew Aladjev) 16 days ago

I've found the source commit 068dd60372d659c97a76224dfce38ed96984cb6f where socklist was introduced. st_table is just hash table, it looks like original code provides global socklist hash table and writes into it without any locks. This code was designed for single thread mode only.

Today we can see that multiple GetCurrentThreadId is processing single global socklist, we need to add locks.

Updated by jeremyevans0 (Jeremy Evans) 11 days ago

  • Status changed from Open to Closed

Updated by puchuu (Andrew Aladjev) 11 days ago

I've prepared backports for ruby_3_0 and ruby_2_7 on github, but ruby_2_6 is not using github, so i am going to attach it here.

Also available in: Atom PDF