From f5af429608481fdd09731cbd8ef8be05d86ca70d Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Thu, 2 Oct 2025 14:52:31 +0200 Subject: [PATCH 1/3] Extract `Crystal::EventLoop#before_close` from `#close` --- src/crystal/event_loop/file_descriptor.cr | 11 ++++++++++- src/crystal/event_loop/libevent.cr | 10 ++++++++-- src/crystal/event_loop/polling.cr | 10 ++++++++-- src/crystal/event_loop/socket.cr | 10 +++++++++- src/crystal/event_loop/wasi.cr | 10 ++++++++-- src/crystal/system/unix/file_descriptor.cr | 1 + src/crystal/system/unix/socket.cr | 1 + src/crystal/system/wasi/socket.cr | 1 + src/crystal/system/win32/file_descriptor.cr | 1 + 9 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/crystal/event_loop/file_descriptor.cr b/src/crystal/event_loop/file_descriptor.cr index a83924b3986e..ff394cf3de84 100644 --- a/src/crystal/event_loop/file_descriptor.cr +++ b/src/crystal/event_loop/file_descriptor.cr @@ -46,7 +46,16 @@ abstract class Crystal::EventLoop # file descriptor. abstract def reopened(file_descriptor : Crystal::System::FileDescriptor) : Nil - # Closes the file descriptor resource. + # Hook to react on the file descriptor after the IO object has been marked + # closed but before calling `#close` to actually close the system fd or + # handle. + # + # On UNIX targets the event loop must resume pending waiters and let them + # fail because the IO object is closed. + def before_close(file_descriptor : Crystal::System::FileDescriptor) : Nil + end + + # Closes the system fd or handle. abstract def close(file_descriptor : Crystal::System::FileDescriptor) : Nil end diff --git a/src/crystal/event_loop/libevent.cr b/src/crystal/event_loop/libevent.cr index d0eef0f88425..33638066caaa 100644 --- a/src/crystal/event_loop/libevent.cr +++ b/src/crystal/event_loop/libevent.cr @@ -178,10 +178,13 @@ class Crystal::EventLoop::LibEvent < Crystal::EventLoop file_descriptor.evented_close end - def close(file_descriptor : Crystal::System::FileDescriptor) : Nil + def before_close(file_descriptor : Crystal::System::FileDescriptor) : Nil # perform cleanup before LibC.close. Using a file descriptor after it has # been closed is never defined and can always lead to undefined results file_descriptor.evented_close + end + + def close(file_descriptor : Crystal::System::FileDescriptor) : Nil file_descriptor.file_descriptor_close end @@ -299,10 +302,13 @@ class Crystal::EventLoop::LibEvent < Crystal::EventLoop end end - def close(socket : ::Socket) : Nil + def before_close(socket : ::Socket) : Nil # perform cleanup before LibC.close. Using a file descriptor after it has # been closed is never defined and can always lead to undefined results socket.evented_close + end + + def close(socket : ::Socket) : Nil socket.socket_close end diff --git a/src/crystal/event_loop/polling.cr b/src/crystal/event_loop/polling.cr index 61c10fe85567..11787f29f53e 100644 --- a/src/crystal/event_loop/polling.cr +++ b/src/crystal/event_loop/polling.cr @@ -232,10 +232,13 @@ abstract class Crystal::EventLoop::Polling < Crystal::EventLoop resume_all(file_descriptor) end - def close(file_descriptor : System::FileDescriptor) : Nil + def before_close(file_descriptor : System::FileDescriptor) : Nil # perform cleanup before LibC.close. Using a file descriptor after it has # been closed is never defined and can always lead to undefined results resume_all(file_descriptor) + end + + def close(file_descriptor : System::FileDescriptor) : Nil file_descriptor.file_descriptor_close end @@ -363,10 +366,13 @@ abstract class Crystal::EventLoop::Polling < Crystal::EventLoop end end - def close(socket : ::Socket) : Nil + def before_close(socket : ::Socket) : Nil # perform cleanup before LibC.close. Using a file descriptor after it has # been closed is never defined and can always lead to undefined results resume_all(socket) + end + + def close(socket : ::Socket) : Nil socket.socket_close end diff --git a/src/crystal/event_loop/socket.cr b/src/crystal/event_loop/socket.cr index f31a1d6cdd70..344b76ed48c4 100644 --- a/src/crystal/event_loop/socket.cr +++ b/src/crystal/event_loop/socket.cr @@ -74,7 +74,15 @@ abstract class Crystal::EventLoop # and the source address. abstract def receive_from(socket : ::Socket, slice : Bytes) : Tuple(Int32, ::Socket::Address) - # Closes the socket. + # Hook to react on the socket after the IO object has been marked closed but + # before calling `#close` to actually close the system socket fd or handle. + # + # On UNIX targets the event loop must resume pending waiters and let them + # fail because the IO object is closed. + def before_close(socket : ::Socket) : Nil + end + + # Closes the system socket fd or handle. abstract def close(socket : ::Socket) : Nil end diff --git a/src/crystal/event_loop/wasi.cr b/src/crystal/event_loop/wasi.cr index 19b2f5e65868..a1e3c928413f 100644 --- a/src/crystal/event_loop/wasi.cr +++ b/src/crystal/event_loop/wasi.cr @@ -80,8 +80,11 @@ class Crystal::EventLoop::Wasi < Crystal::EventLoop raise NotImplementedError.new("Crystal::EventLoop#reopened(FileDescriptor)") end - def close(file_descriptor : Crystal::System::FileDescriptor) : Nil + def before_close(file_descriptor : Crystal::System::FileDescriptor) : Nil file_descriptor.evented_close + end + + def close(file_descriptor : Crystal::System::FileDescriptor) : Nil file_descriptor.file_descriptor_close end @@ -133,8 +136,11 @@ class Crystal::EventLoop::Wasi < Crystal::EventLoop raise NotImplementedError.new "Crystal::Wasi::EventLoop#accept" end - def close(socket : ::Socket) : Nil + def before_close(socket : ::Socket) : Nil socket.evented_close + end + + def close(socket : ::Socket) : Nil socket.socket_close end diff --git a/src/crystal/system/unix/file_descriptor.cr b/src/crystal/system/unix/file_descriptor.cr index 1cb083cfe9cc..78d1f6822023 100644 --- a/src/crystal/system/unix/file_descriptor.cr +++ b/src/crystal/system/unix/file_descriptor.cr @@ -134,6 +134,7 @@ module Crystal::System::FileDescriptor end private def system_close + event_loop.before_close(self) event_loop.close(self) end diff --git a/src/crystal/system/unix/socket.cr b/src/crystal/system/unix/socket.cr index 2ef8e58272c2..5f60f8c7e92a 100644 --- a/src/crystal/system/unix/socket.cr +++ b/src/crystal/system/unix/socket.cr @@ -224,6 +224,7 @@ module Crystal::System::Socket end private def system_close + event_loop.before_close(self) event_loop.close(self) end diff --git a/src/crystal/system/wasi/socket.cr b/src/crystal/system/wasi/socket.cr index 4d29cb373868..5ff1c5ad143f 100644 --- a/src/crystal/system/wasi/socket.cr +++ b/src/crystal/system/wasi/socket.cr @@ -152,6 +152,7 @@ module Crystal::System::Socket end private def system_close + event_loop.before_close(self) event_loop.close(self) end diff --git a/src/crystal/system/win32/file_descriptor.cr b/src/crystal/system/win32/file_descriptor.cr index 0d841db510fd..13f3dd9fe419 100644 --- a/src/crystal/system/win32/file_descriptor.cr +++ b/src/crystal/system/win32/file_descriptor.cr @@ -216,6 +216,7 @@ module Crystal::System::FileDescriptor end private def system_close + event_loop.before_close(self) event_loop.close(self) end From 28bc979ee5697e5702141c47029f88e1cc967eb1 Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Thu, 30 Oct 2025 18:17:45 +0100 Subject: [PATCH 2/3] Rename #before_close to #shutdown --- src/crystal/event_loop/file_descriptor.cr | 13 ++++++------- src/crystal/event_loop/iocp.cr | 6 ++++++ src/crystal/event_loop/libevent.cr | 4 ++-- src/crystal/event_loop/polling.cr | 4 ++-- src/crystal/event_loop/socket.cr | 12 ++++++------ src/crystal/event_loop/wasi.cr | 10 ++++++++-- src/crystal/system/unix/file_descriptor.cr | 2 +- src/crystal/system/unix/socket.cr | 2 +- src/crystal/system/wasi/socket.cr | 2 +- src/crystal/system/win32/file_descriptor.cr | 2 +- 10 files changed, 34 insertions(+), 23 deletions(-) diff --git a/src/crystal/event_loop/file_descriptor.cr b/src/crystal/event_loop/file_descriptor.cr index ff394cf3de84..a28443966870 100644 --- a/src/crystal/event_loop/file_descriptor.cr +++ b/src/crystal/event_loop/file_descriptor.cr @@ -46,14 +46,13 @@ abstract class Crystal::EventLoop # file descriptor. abstract def reopened(file_descriptor : Crystal::System::FileDescriptor) : Nil - # Hook to react on the file descriptor after the IO object has been marked - # closed but before calling `#close` to actually close the system fd or - # handle. + # Internal shutdown of the file descriptor. Called after the + # IO::FileDescriptor has been marked closed but before calling `#close` to + # actually close the system fd or handle. # - # On UNIX targets the event loop must resume pending waiters and let them - # fail because the IO object is closed. - def before_close(file_descriptor : Crystal::System::FileDescriptor) : Nil - end + # Implementations shall resume all pending waiters and let them fail because + # the IO has been closed. + abstract def shutdown(file_descriptor : Crystal::System::FileDescriptor) : Nil # Closes the system fd or handle. abstract def close(file_descriptor : Crystal::System::FileDescriptor) : Nil diff --git a/src/crystal/event_loop/iocp.cr b/src/crystal/event_loop/iocp.cr index e6885ed9ac82..a8c329d0cf8f 100644 --- a/src/crystal/event_loop/iocp.cr +++ b/src/crystal/event_loop/iocp.cr @@ -307,6 +307,9 @@ class Crystal::EventLoop::IOCP < Crystal::EventLoop raise NotImplementedError.new("Crystal::System::IOCP#reopened(FileDescriptor)") end + def shutdown(file_descriptor : Crystal::System::FileDescriptor) : Nil + end + def close(file_descriptor : Crystal::System::FileDescriptor) : Nil LibC.CancelIoEx(file_descriptor.windows_handle, nil) unless file_descriptor.system_blocking? file_descriptor.file_descriptor_close @@ -445,6 +448,9 @@ class Crystal::EventLoop::IOCP < Crystal::EventLoop end end + def shutdown(socket : ::Socket) : Nil + end + def close(socket : ::Socket) : Nil raise NotImplementedError.new("Crystal::System::IOCP#close(Socket)") end diff --git a/src/crystal/event_loop/libevent.cr b/src/crystal/event_loop/libevent.cr index 33638066caaa..a3bd0f957ab4 100644 --- a/src/crystal/event_loop/libevent.cr +++ b/src/crystal/event_loop/libevent.cr @@ -178,7 +178,7 @@ class Crystal::EventLoop::LibEvent < Crystal::EventLoop file_descriptor.evented_close end - def before_close(file_descriptor : Crystal::System::FileDescriptor) : Nil + def shutdown(file_descriptor : Crystal::System::FileDescriptor) : Nil # perform cleanup before LibC.close. Using a file descriptor after it has # been closed is never defined and can always lead to undefined results file_descriptor.evented_close @@ -302,7 +302,7 @@ class Crystal::EventLoop::LibEvent < Crystal::EventLoop end end - def before_close(socket : ::Socket) : Nil + def shutdown(socket : ::Socket) : Nil # perform cleanup before LibC.close. Using a file descriptor after it has # been closed is never defined and can always lead to undefined results socket.evented_close diff --git a/src/crystal/event_loop/polling.cr b/src/crystal/event_loop/polling.cr index 11787f29f53e..1c2bf76742b1 100644 --- a/src/crystal/event_loop/polling.cr +++ b/src/crystal/event_loop/polling.cr @@ -232,7 +232,7 @@ abstract class Crystal::EventLoop::Polling < Crystal::EventLoop resume_all(file_descriptor) end - def before_close(file_descriptor : System::FileDescriptor) : Nil + def shutdown(file_descriptor : System::FileDescriptor) : Nil # perform cleanup before LibC.close. Using a file descriptor after it has # been closed is never defined and can always lead to undefined results resume_all(file_descriptor) @@ -366,7 +366,7 @@ abstract class Crystal::EventLoop::Polling < Crystal::EventLoop end end - def before_close(socket : ::Socket) : Nil + def shutdown(socket : ::Socket) : Nil # perform cleanup before LibC.close. Using a file descriptor after it has # been closed is never defined and can always lead to undefined results resume_all(socket) diff --git a/src/crystal/event_loop/socket.cr b/src/crystal/event_loop/socket.cr index 344b76ed48c4..62022f001a81 100644 --- a/src/crystal/event_loop/socket.cr +++ b/src/crystal/event_loop/socket.cr @@ -74,13 +74,13 @@ abstract class Crystal::EventLoop # and the source address. abstract def receive_from(socket : ::Socket, slice : Bytes) : Tuple(Int32, ::Socket::Address) - # Hook to react on the socket after the IO object has been marked closed but - # before calling `#close` to actually close the system socket fd or handle. + # Internal shutdown of the socket. Called after the Socket has been marked + # closed but before calling `#close` to actually close the system socket fd + # or handle. # - # On UNIX targets the event loop must resume pending waiters and let them - # fail because the IO object is closed. - def before_close(socket : ::Socket) : Nil - end + # Implementations shall resume all pending waiters and let them fail because + # the IO has been closed. They don't have to call the `shutdown` syscall. + abstract def shutdown(socket : ::Socket) : Nil # Closes the system socket fd or handle. abstract def close(socket : ::Socket) : Nil diff --git a/src/crystal/event_loop/wasi.cr b/src/crystal/event_loop/wasi.cr index a1e3c928413f..2eac2f6adc57 100644 --- a/src/crystal/event_loop/wasi.cr +++ b/src/crystal/event_loop/wasi.cr @@ -80,10 +80,13 @@ class Crystal::EventLoop::Wasi < Crystal::EventLoop raise NotImplementedError.new("Crystal::EventLoop#reopened(FileDescriptor)") end - def before_close(file_descriptor : Crystal::System::FileDescriptor) : Nil + def shutdown(file_descriptor : Crystal::System::FileDescriptor) : Nil file_descriptor.evented_close end + def shutdown(file_descriptor : Crystal::System::FileDescriptor) : Nil + end + def close(file_descriptor : Crystal::System::FileDescriptor) : Nil file_descriptor.file_descriptor_close end @@ -136,10 +139,13 @@ class Crystal::EventLoop::Wasi < Crystal::EventLoop raise NotImplementedError.new "Crystal::Wasi::EventLoop#accept" end - def before_close(socket : ::Socket) : Nil + def shutdown(socket : ::Socket) : Nil socket.evented_close end + def shutdown(socket : ::Socket) : Nil + end + def close(socket : ::Socket) : Nil socket.socket_close end diff --git a/src/crystal/system/unix/file_descriptor.cr b/src/crystal/system/unix/file_descriptor.cr index 78d1f6822023..003d036985e6 100644 --- a/src/crystal/system/unix/file_descriptor.cr +++ b/src/crystal/system/unix/file_descriptor.cr @@ -134,7 +134,7 @@ module Crystal::System::FileDescriptor end private def system_close - event_loop.before_close(self) + event_loop.shutdown(self) event_loop.close(self) end diff --git a/src/crystal/system/unix/socket.cr b/src/crystal/system/unix/socket.cr index 5f60f8c7e92a..f0205930b1c1 100644 --- a/src/crystal/system/unix/socket.cr +++ b/src/crystal/system/unix/socket.cr @@ -224,7 +224,7 @@ module Crystal::System::Socket end private def system_close - event_loop.before_close(self) + event_loop.shutdown(self) event_loop.close(self) end diff --git a/src/crystal/system/wasi/socket.cr b/src/crystal/system/wasi/socket.cr index 5ff1c5ad143f..baf5cbfb5414 100644 --- a/src/crystal/system/wasi/socket.cr +++ b/src/crystal/system/wasi/socket.cr @@ -152,7 +152,7 @@ module Crystal::System::Socket end private def system_close - event_loop.before_close(self) + event_loop.shutdown(self) event_loop.close(self) end diff --git a/src/crystal/system/win32/file_descriptor.cr b/src/crystal/system/win32/file_descriptor.cr index 13f3dd9fe419..91eb0b99ab27 100644 --- a/src/crystal/system/win32/file_descriptor.cr +++ b/src/crystal/system/win32/file_descriptor.cr @@ -216,7 +216,7 @@ module Crystal::System::FileDescriptor end private def system_close - event_loop.before_close(self) + event_loop.shutdown(self) event_loop.close(self) end From 5328b48bac00e371a45d4af3e45566c8a4e812b5 Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Thu, 30 Oct 2025 22:13:02 +0100 Subject: [PATCH 3/3] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Johannes Müller --- src/crystal/event_loop/file_descriptor.cr | 2 +- src/crystal/event_loop/socket.cr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/crystal/event_loop/file_descriptor.cr b/src/crystal/event_loop/file_descriptor.cr index a28443966870..54840fe138be 100644 --- a/src/crystal/event_loop/file_descriptor.cr +++ b/src/crystal/event_loop/file_descriptor.cr @@ -47,7 +47,7 @@ abstract class Crystal::EventLoop abstract def reopened(file_descriptor : Crystal::System::FileDescriptor) : Nil # Internal shutdown of the file descriptor. Called after the - # IO::FileDescriptor has been marked closed but before calling `#close` to + # `IO::FileDescriptor` has been marked closed but before calling `#close` to # actually close the system fd or handle. # # Implementations shall resume all pending waiters and let them fail because diff --git a/src/crystal/event_loop/socket.cr b/src/crystal/event_loop/socket.cr index 62022f001a81..ed7ac7adeda0 100644 --- a/src/crystal/event_loop/socket.cr +++ b/src/crystal/event_loop/socket.cr @@ -74,7 +74,7 @@ abstract class Crystal::EventLoop # and the source address. abstract def receive_from(socket : ::Socket, slice : Bytes) : Tuple(Int32, ::Socket::Address) - # Internal shutdown of the socket. Called after the Socket has been marked + # Internal shutdown of the socket. Called after the `Socket` has been marked # closed but before calling `#close` to actually close the system socket fd # or handle. #