Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: make Crystal::EventLoop#remove(io) a class method #15282

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions src/crystal/event_loop.cr
Original file line number Diff line number Diff line change
@@ -1,26 +1,30 @@
abstract class Crystal::EventLoop
# Creates an event loop instance
def self.create : self
def self.backend_class
{% if flag?(:wasi) %}
Crystal::EventLoop::Wasi.new
Crystal::EventLoop::Wasi
{% elsif flag?(:unix) %}
# TODO: enable more targets by default (need manual tests or fixes)
{% if flag?("evloop=libevent") %}
Crystal::EventLoop::LibEvent.new
Crystal::EventLoop::LibEvent
{% elsif flag?("evloop=epoll") || flag?(:android) || flag?(:linux) %}
Crystal::EventLoop::Epoll.new
Crystal::EventLoop::Epoll
{% elsif flag?("evloop=kqueue") || flag?(:darwin) || flag?(:freebsd) %}
Crystal::EventLoop::Kqueue.new
Crystal::EventLoop::Kqueue
{% else %}
Crystal::EventLoop::LibEvent.new
Crystal::EventLoop::LibEvent
{% end %}
{% elsif flag?(:win32) %}
Crystal::EventLoop::IOCP.new
Crystal::EventLoop::IOCP
{% else %}
{% raise "Event loop not supported" %}
{% end %}
end

# Creates an event loop instance
def self.create : self
backend_class.new
end

@[AlwaysInline]
def self.current : self
Crystal::Scheduler.event_loop
Expand Down
21 changes: 14 additions & 7 deletions src/crystal/event_loop/file_descriptor.cr
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,20 @@ abstract class Crystal::EventLoop

# Closes the file descriptor resource.
abstract def close(file_descriptor : Crystal::System::FileDescriptor) : Nil
end

# Removes the file descriptor from the event loop. Can be used to free up
# memory resources associated with the file descriptor, as well as removing
# the file descriptor from kernel data structures.
#
# Called by `::IO::FileDescriptor#finalize` before closing the file
# descriptor. Errors shall be silently ignored.
abstract def remove(file_descriptor : Crystal::System::FileDescriptor) : Nil
# Removes the file descriptor from the event loop. Can be used to free up
# memory resources associated with the file descriptor, as well as removing
# the file descriptor from kernel data structures.
#
# Called by `::IO::FileDescriptor#finalize` before closing the file
# descriptor. Errors shall be silently ignored.
def self.remove(file_descriptor : Crystal::System::FileDescriptor) : Nil
backend_class.remove_impl(file_descriptor)
end

# Actual implementation for `.remove`. Must be implemented on a subclass of
# `Crystal::EventLoop` when needed.
protected def self.remove_impl(file_descriptor : Crystal::System::FileDescriptor) : Nil
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
protected def self.remove_impl(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
protected def self.remove_impl(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
21 changes: 14 additions & 7 deletions src/crystal/event_loop/socket.cr
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,20 @@ abstract class Crystal::EventLoop

# Closes the socket.
abstract def close(socket : ::Socket) : Nil
end

# Removes the socket from the event loop. Can be used to free up memory
# resources associated with the socket, as well as removing the socket from
# kernel data structures.
#
# Called by `::Socket#finalize` before closing the socket. Errors shall be
# silently ignored.
abstract def remove(socket : ::Socket) : Nil
# Removes the socket from the event loop. Can be used to free up memory
# resources associated with the socket, as well as removing the socket from
# kernel data structures.
#
# Called by `::Socket#finalize` before closing the socket. Errors shall be
# silently ignored.
def self.remove(socket : ::Socket) : Nil
backend_class.remove_impl(socket)
end

# Actual implementation for `.remove`. Must be implemented on a subclass of
# `Crystal::EventLoop` when needed.
protected def self.remove_impl(socket : ::Socket) : Nil
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)
Crystal::EventLoop.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)
Crystal::EventLoop.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))
Crystal::EventLoop.remove(self)
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))
Crystal::EventLoop.remove(self)
socket_close { } # ignore error
end

Expand Down
Loading