From 689e8515e1fd87991805c9ca037971088463728e Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 14 Jan 2025 18:27:54 +1300 Subject: [PATCH 1/2] Save and restore `errno` in `IO_Event_Interrupt_signal`. --- ext/io/event/interrupt.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/ext/io/event/interrupt.c b/ext/io/event/interrupt.c index da6f8cd..09182cc 100644 --- a/ext/io/event/interrupt.c +++ b/ext/io/event/interrupt.c @@ -92,24 +92,40 @@ void IO_Event_Interrupt_close(struct IO_Event_Interrupt *interrupt) void IO_Event_Interrupt_signal(struct IO_Event_Interrupt *interrupt) { + // This function can be used to wake up the selector when a mutex is unlocked. Because that can happen in some contexts where `errno` is not preserved, we save and restore it. We should probably fix the root cause of this issue, by ensuring that `errno` is saved immediately after the system call that can set it. + int saved_errno = errno; + ssize_t result = write(interrupt->descriptor[1], ".", 1); if (result == -1) { - if (errno == EAGAIN || errno == EWOULDBLOCK) return; - - rb_sys_fail("IO_Event_Interrupt_signal:write"); + if (errno == EAGAIN || errno == EWOULDBLOCK) { + // If we can't write to the pipe, it means the other end is full. In that case, we can be sure that the other end has already been woken up or is about to be woken up. + break; + } else { + errno = saved_errno; + rb_sys_fail("IO_Event_Interrupt_signal:write"); + } } + + errno = saved_errno; } void IO_Event_Interrupt_clear(struct IO_Event_Interrupt *interrupt) { + int saved_errno = errno; + char buffer[128]; ssize_t result = read(interrupt->descriptor[0], buffer, sizeof(buffer)); if (result == -1) { - if (errno == EAGAIN || errno == EWOULDBLOCK) return; - - rb_sys_fail("IO_Event_Interrupt_clear:read"); + if (errno == EAGAIN || errno == EWOULDBLOCK) { + // Ignore. + } else { + errno = saved_errno; + rb_sys_fail("IO_Event_Interrupt_clear:read"); + } } + + errno = saved_errno; } #endif From 3dbb5860e824832fe38e3cb0c4f477164b22a7a8 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 14 Jan 2025 18:58:30 +1300 Subject: [PATCH 2/2] Apply suggestions from code review --- ext/io/event/interrupt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/io/event/interrupt.c b/ext/io/event/interrupt.c index 09182cc..20a62a7 100644 --- a/ext/io/event/interrupt.c +++ b/ext/io/event/interrupt.c @@ -92,7 +92,7 @@ void IO_Event_Interrupt_close(struct IO_Event_Interrupt *interrupt) void IO_Event_Interrupt_signal(struct IO_Event_Interrupt *interrupt) { - // This function can be used to wake up the selector when a mutex is unlocked. Because that can happen in some contexts where `errno` is not preserved, we save and restore it. We should probably fix the root cause of this issue, by ensuring that `errno` is saved immediately after the system call that can set it. + // This function can be used to wake up the selector when a mutex is unlocked. Because that can happen in some contexts where `errno` should be preserved, we save and restore it. We should probably fix the root cause of this issue, by ensuring that `errno` is saved immediately after the system call that can set it. int saved_errno = errno; ssize_t result = write(interrupt->descriptor[1], ".", 1);