Skip to content

Commit 5b60098

Browse files
KJ Tsanaktsidisk0kubun
authored andcommitted
Ensure fiber scheduler is woken up when close interrupts read
If one thread is reading and another closes that socket, the close blocks waiting for the read to abort cleanly. This ensures that Ruby is totally done with the file descriptor _BEFORE_ we tell the OS to close and potentially re-use it. When the read is correctly terminated, the close should be unblocked. That currently works if closing is happening on a thread, but if it's happening on a fiber with a fiber scheduler, it does NOT work. This patch ensures that if the close happened in a fiber scheduled thread, that the scheduler is notified that the fiber is unblocked. [Bug #20723]
1 parent 4e59e7d commit 5b60098

File tree

3 files changed

+51
-1
lines changed

3 files changed

+51
-1
lines changed

internal/thread.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ int rb_thread_wait_for_single_fd(int fd, int events, struct timeval * timeout);
5757
struct rb_io_close_wait_list {
5858
struct ccan_list_head pending_fd_users;
5959
VALUE closing_thread;
60+
VALUE closing_fiber;
6061
VALUE wakeup_mutex;
6162
};
6263
int rb_notify_fd_close(int fd, struct rb_io_close_wait_list *busy);

test/fiber/test_io.rb

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,4 +234,47 @@ def test_backquote
234234

235235
assert_equal "ok\n", result
236236
end
237+
238+
# Tests for https://bugs.ruby-lang.org/issues/20723 which would
239+
# otherwise deadlock this test.
240+
def test_close_while_reading_on_thread
241+
# Windows has UNIXSocket, but only with VS 2019+
242+
omit "UNIXSocket is not defined!" unless defined?(UNIXSocket)
243+
244+
i, o = Socket.pair(:UNIX, :STREAM)
245+
if RUBY_PLATFORM=~/mswin|mingw/
246+
i.nonblock = true
247+
o.nonblock = true
248+
end
249+
250+
message = nil
251+
252+
reading_thread = Thread.new do
253+
Thread.current.report_on_exception = false
254+
i.wait_readable
255+
end
256+
257+
fs_thread = Thread.new do
258+
# Wait until the reading thread is blocked on read:
259+
Thread.pass until reading_thread.status == "sleep"
260+
261+
scheduler = Scheduler.new
262+
Fiber.set_scheduler scheduler
263+
Fiber.schedule do
264+
i.close
265+
end
266+
end
267+
268+
assert_raise(IOError) { reading_thread.join }
269+
refute_nil fs_thread.join(5), "expected thread to terminate within 5 seconds"
270+
271+
assert_predicate(i, :closed?)
272+
ensure
273+
fs_thread&.kill
274+
fs_thread&.join rescue nil
275+
reading_thread&.kill
276+
reading_thread&.join rescue nil
277+
i&.close
278+
o&.close
279+
end
237280
end

thread.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1698,7 +1698,12 @@ thread_io_wake_pending_closer(struct waiting_fd *wfd)
16981698
RB_VM_LOCK_LEAVE();
16991699

17001700
if (has_waiter) {
1701-
rb_thread_wakeup(wfd->busy->closing_thread);
1701+
rb_thread_t *th = rb_thread_ptr(wfd->busy->closing_thread);
1702+
if (th->scheduler != Qnil) {
1703+
rb_fiber_scheduler_unblock(th->scheduler, wfd->busy->closing_thread, wfd->busy->closing_fiber);
1704+
} else {
1705+
rb_thread_wakeup(wfd->busy->closing_thread);
1706+
}
17021707
rb_mutex_unlock(wfd->busy->wakeup_mutex);
17031708
}
17041709
}
@@ -2609,6 +2614,7 @@ rb_notify_fd_close(int fd, struct rb_io_close_wait_list *busy)
26092614

26102615
has_any = !ccan_list_empty(&busy->pending_fd_users);
26112616
busy->closing_thread = rb_thread_current();
2617+
busy->closing_fiber = rb_fiber_current();
26122618
wakeup_mutex = Qnil;
26132619
if (has_any) {
26142620
wakeup_mutex = rb_mutex_new();

0 commit comments

Comments
 (0)