Skip to content

Commit

Permalink
Fix: make Crystal::EventLoop#remove(io) a class method
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ysbaddaden committed Dec 16, 2024
1 parent 46459d6 commit 31f39d7
Show file tree
Hide file tree
Showing 10 changed files with 11 additions and 27 deletions.
3 changes: 2 additions & 1 deletion src/crystal/event_loop/file_descriptor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 0 additions & 6 deletions src/crystal/event_loop/iocp.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -314,7 +311,4 @@ class Crystal::EventLoop::IOCP < Crystal::EventLoop

def close(socket : ::Socket) : Nil
end

def remove(socket : ::Socket) : Nil
end
end
6 changes: 0 additions & 6 deletions src/crystal/event_loop/libevent.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/crystal/event_loop/polling.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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|
Expand Down
3 changes: 2 additions & 1 deletion src/crystal/event_loop/socket.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 0 additions & 6 deletions src/crystal/event_loop/wasi.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/crystal/system/unix/process.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/crystal/system/unix/signal.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/io/file_descriptor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Check failure on line 258 in src/io/file_descriptor.cr

View workflow job for this annotation

GitHub Actions / x86_64-mingw-w64-cross-compile

undefined method 'remove' for Crystal::EventLoop::IOCP.class (compile-time type is Crystal::EventLoop+.class)

Check failure on line 258 in src/io/file_descriptor.cr

View workflow job for this annotation

GitHub Actions / x86_64-windows-release / build

undefined method 'remove' for Crystal::EventLoop::IOCP.class (compile-time type is Crystal::EventLoop+.class)
file_descriptor_close { } # ignore error
end

Expand Down
2 changes: 1 addition & 1 deletion src/socket.cr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 31f39d7

Please sign in to comment.