From 31f39d767b225acd7db44bd76296450f08430afc Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Mon, 16 Dec 2024 11:09:04 +0100 Subject: [PATCH] Fix: make Crystal::EventLoop#remove(io) a class method The method is called from IO::FileDescriptor and Socket finalizers, which means they can be run from any thread during GC collections, yet calling an instance method means accessing the current event loop, which may have not been instantiated yet for the thread. --- src/crystal/event_loop/file_descriptor.cr | 3 ++- src/crystal/event_loop/iocp.cr | 6 ------ src/crystal/event_loop/libevent.cr | 6 ------ src/crystal/event_loop/polling.cr | 6 +++--- src/crystal/event_loop/socket.cr | 3 ++- src/crystal/event_loop/wasi.cr | 6 ------ src/crystal/system/unix/process.cr | 2 +- src/crystal/system/unix/signal.cr | 2 +- src/io/file_descriptor.cr | 2 +- src/socket.cr | 2 +- 10 files changed, 11 insertions(+), 27 deletions(-) diff --git a/src/crystal/event_loop/file_descriptor.cr b/src/crystal/event_loop/file_descriptor.cr index 5fb6cbb95cb0..c0df2e5aefff 100644 --- a/src/crystal/event_loop/file_descriptor.cr +++ b/src/crystal/event_loop/file_descriptor.cr @@ -26,6 +26,7 @@ abstract class Crystal::EventLoop # # Called by `::IO::FileDescriptor#finalize` before closing the file # descriptor. Errors shall be silently ignored. - abstract def remove(file_descriptor : Crystal::System::FileDescriptor) : Nil + def self.remove(file_descriptor : Crystal::System::FileDescriptor) : Nil + end end end diff --git a/src/crystal/event_loop/iocp.cr b/src/crystal/event_loop/iocp.cr index 5628e99121b1..6e4175e3daee 100644 --- a/src/crystal/event_loop/iocp.cr +++ b/src/crystal/event_loop/iocp.cr @@ -201,9 +201,6 @@ class Crystal::EventLoop::IOCP < Crystal::EventLoop LibC.CancelIoEx(file_descriptor.windows_handle, nil) unless file_descriptor.system_blocking? end - def remove(file_descriptor : Crystal::System::FileDescriptor) : Nil - end - private def wsa_buffer(bytes) wsabuf = LibC::WSABUF.new wsabuf.len = bytes.size @@ -314,7 +311,4 @@ class Crystal::EventLoop::IOCP < Crystal::EventLoop def close(socket : ::Socket) : Nil end - - def remove(socket : ::Socket) : Nil - end end diff --git a/src/crystal/event_loop/libevent.cr b/src/crystal/event_loop/libevent.cr index 7b45939bd537..9c0b3d33b15c 100644 --- a/src/crystal/event_loop/libevent.cr +++ b/src/crystal/event_loop/libevent.cr @@ -98,9 +98,6 @@ class Crystal::EventLoop::LibEvent < Crystal::EventLoop file_descriptor.evented_close end - def remove(file_descriptor : Crystal::System::FileDescriptor) : Nil - end - def read(socket : ::Socket, slice : Bytes) : Int32 evented_read(socket, "Error reading socket") do LibC.recv(socket.fd, slice, slice.size, 0).to_i32 @@ -194,9 +191,6 @@ class Crystal::EventLoop::LibEvent < Crystal::EventLoop socket.evented_close end - def remove(socket : ::Socket) : Nil - end - def evented_read(target, errno_msg : String, &) : Int32 loop do bytes_read = yield diff --git a/src/crystal/event_loop/polling.cr b/src/crystal/event_loop/polling.cr index 774cc7060715..510b45909b04 100644 --- a/src/crystal/event_loop/polling.cr +++ b/src/crystal/event_loop/polling.cr @@ -164,7 +164,7 @@ abstract class Crystal::EventLoop::Polling < Crystal::EventLoop evented_close(file_descriptor) end - def remove(file_descriptor : System::FileDescriptor) : Nil + def self.remove(file_descriptor : System::FileDescriptor) : Nil internal_remove(file_descriptor) end @@ -267,7 +267,7 @@ abstract class Crystal::EventLoop::Polling < Crystal::EventLoop evented_close(socket) end - def remove(socket : ::Socket) : Nil + def self.remove(socket : ::Socket) : Nil internal_remove(socket) end @@ -317,7 +317,7 @@ abstract class Crystal::EventLoop::Polling < Crystal::EventLoop end end - private def internal_remove(io) + private def self.internal_remove(io) return unless (index = io.__evloop_data).valid? Polling.arena.free(index) do |pd| diff --git a/src/crystal/event_loop/socket.cr b/src/crystal/event_loop/socket.cr index 03b556b3be96..cd9926bf6ee3 100644 --- a/src/crystal/event_loop/socket.cr +++ b/src/crystal/event_loop/socket.cr @@ -69,6 +69,7 @@ abstract class Crystal::EventLoop # # Called by `::Socket#finalize` before closing the socket. Errors shall be # silently ignored. - abstract def remove(socket : ::Socket) : Nil + def self.remove(socket : ::Socket) : Nil + end end end diff --git a/src/crystal/event_loop/wasi.cr b/src/crystal/event_loop/wasi.cr index a91c469f406c..08781b4fb950 100644 --- a/src/crystal/event_loop/wasi.cr +++ b/src/crystal/event_loop/wasi.cr @@ -53,9 +53,6 @@ class Crystal::EventLoop::Wasi < Crystal::EventLoop file_descriptor.evented_close end - def remove(file_descriptor : Crystal::System::FileDescriptor) : Nil - end - def read(socket : ::Socket, slice : Bytes) : Int32 evented_read(socket, "Error reading socket") do LibC.recv(socket.fd, slice, slice.size, 0).to_i32 @@ -88,9 +85,6 @@ class Crystal::EventLoop::Wasi < Crystal::EventLoop socket.evented_close end - def remove(socket : ::Socket) : Nil - end - def evented_read(target, errno_msg : String, &) : Int32 loop do bytes_read = yield diff --git a/src/crystal/system/unix/process.cr b/src/crystal/system/unix/process.cr index 875d834bb266..7330cb28b3c7 100644 --- a/src/crystal/system/unix/process.cr +++ b/src/crystal/system/unix/process.cr @@ -352,7 +352,7 @@ struct Crystal::System::Process private def self.reopen_io(src_io : IO::FileDescriptor, dst_io : IO::FileDescriptor) if src_io.closed? - Crystal::EventLoop.current.remove(dst_io) + typeof(Crystal::EventLoop.current).remove(dst_io) dst_io.file_descriptor_close else src_io = to_real_fd(src_io) diff --git a/src/crystal/system/unix/signal.cr b/src/crystal/system/unix/signal.cr index 802cb418db15..9eb3db7bfbda 100644 --- a/src/crystal/system/unix/signal.cr +++ b/src/crystal/system/unix/signal.cr @@ -111,7 +111,7 @@ module Crystal::System::Signal # descriptors of the parent process and send it received signals. def self.after_fork @@pipe.each do |pipe_io| - Crystal::EventLoop.current.remove(pipe_io) + typeof(Crystal::EventLoop.current).remove(pipe_io) pipe_io.file_descriptor_close { } end ensure diff --git a/src/io/file_descriptor.cr b/src/io/file_descriptor.cr index a9b303b4b58c..1689081e09c5 100644 --- a/src/io/file_descriptor.cr +++ b/src/io/file_descriptor.cr @@ -255,7 +255,7 @@ class IO::FileDescriptor < IO def finalize return if closed? || !close_on_finalize? - event_loop?.try(&.remove(self)) + typeof(Crystal::EventLoop.current).remove(self) file_descriptor_close { } # ignore error end diff --git a/src/socket.cr b/src/socket.cr index e97deea9eb04..5801fc94db88 100644 --- a/src/socket.cr +++ b/src/socket.cr @@ -430,7 +430,7 @@ class Socket < IO def finalize return if closed? - event_loop?.try(&.remove(self)) + typeof(Crystal::EventLoop.current).remove(self) socket_close { } # ignore error end