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]
This commit is contained in:
KJ Tsanaktsidis 2024-09-13 17:40:30 +10:00 committed by KJ Tsanaktsidis
parent 50d4840bd9
commit e08d5239b6
Notes: git 2024-09-17 00:12:01 +00:00
3 changed files with 51 additions and 1 deletions

View File

@ -60,6 +60,7 @@ int rb_thread_wait_for_single_fd(int fd, int events, struct timeval * timeout);
struct rb_io_close_wait_list {
struct ccan_list_head pending_fd_users;
VALUE closing_thread;
VALUE closing_fiber;
VALUE wakeup_mutex;
};
int rb_notify_fd_close(int fd, struct rb_io_close_wait_list *busy);

View File

@ -234,4 +234,47 @@ class TestFiberIO < Test::Unit::TestCase
assert_equal "ok\n", result
end
# Tests for https://bugs.ruby-lang.org/issues/20723 which would
# otherwise deadlock this test.
def test_close_while_reading_on_thread
# Windows has UNIXSocket, but only with VS 2019+
omit "UNIXSocket is not defined!" unless defined?(UNIXSocket)
i, o = Socket.pair(:UNIX, :STREAM)
if RUBY_PLATFORM=~/mswin|mingw/
i.nonblock = true
o.nonblock = true
end
message = nil
reading_thread = Thread.new do
Thread.current.report_on_exception = false
i.wait_readable
end
fs_thread = Thread.new do
# Wait until the reading thread is blocked on read:
Thread.pass until reading_thread.status == "sleep"
scheduler = Scheduler.new
Fiber.set_scheduler scheduler
Fiber.schedule do
i.close
end
end
assert_raise(IOError) { reading_thread.join }
refute_nil fs_thread.join(5), "expected thread to terminate within 5 seconds"
assert_predicate(i, :closed?)
ensure
fs_thread&.kill
fs_thread&.join rescue nil
reading_thread&.kill
reading_thread&.join rescue nil
i&.close
o&.close
end
end

View File

@ -1698,7 +1698,12 @@ thread_io_wake_pending_closer(struct waiting_fd *wfd)
RB_VM_LOCK_LEAVE();
if (has_waiter) {
rb_thread_wakeup(wfd->busy->closing_thread);
rb_thread_t *th = rb_thread_ptr(wfd->busy->closing_thread);
if (th->scheduler != Qnil) {
rb_fiber_scheduler_unblock(th->scheduler, wfd->busy->closing_thread, wfd->busy->closing_fiber);
} else {
rb_thread_wakeup(wfd->busy->closing_thread);
}
rb_mutex_unlock(wfd->busy->wakeup_mutex);
}
}
@ -2625,6 +2630,7 @@ rb_notify_fd_close(int fd, struct rb_io_close_wait_list *busy)
has_any = !ccan_list_empty(&busy->pending_fd_users);
busy->closing_thread = rb_thread_current();
busy->closing_fiber = rb_fiber_current();
wakeup_mutex = Qnil;
if (has_any) {
wakeup_mutex = rb_mutex_new();