From 07d5ead61791b015da1d53b569ddb22bf0f5d433 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Wed, 25 Nov 2020 23:30:38 +0100 Subject: [PATCH 1/3] Causes the event loop to exit on Posix when being interrupted by a signal. Previously, the loop logic would simply retry the wait until the given timeout was actually reached (or an event got received), which could be forever. This change makes sure that after an interrupt the higher level code will get a chance to run to possibly handle the effects of the signal. This is a fix for vibe-d/vibe-core#205. --- source/eventcore/drivers/posix/epoll.d | 11 ++++++++++- source/eventcore/drivers/posix/kqueue.d | 11 ++++++++++- source/eventcore/drivers/posix/select.d | 11 ++++++++++- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/source/eventcore/drivers/posix/epoll.d b/source/eventcore/drivers/posix/epoll.d index b6a5767..1abb020 100644 --- a/source/eventcore/drivers/posix/epoll.d +++ b/source/eventcore/drivers/posix/epoll.d @@ -57,7 +57,16 @@ final class EpollEventLoop : PosixEventLoop { if (evt.events & EPOLLOUT) notify!(EventType.write)(fd); } return true; - } else return false; + } else { + // NOTE: In particular, EINTR needs to cause true to be returned + // here, so that user code has a chance to handle any effects + // of the signal handler before waiting again. + // + // Other errors are very likely to to reoccur for the next + // loop iteration, so there is no value in attempting to + // wait again. + return ret < 0; + } } override void dispose() diff --git a/source/eventcore/drivers/posix/kqueue.d b/source/eventcore/drivers/posix/kqueue.d index 3692e83..800f4b9 100644 --- a/source/eventcore/drivers/posix/kqueue.d +++ b/source/eventcore/drivers/posix/kqueue.d @@ -85,7 +85,16 @@ abstract class KqueueEventLoopBase : PosixEventLoop { // EV_SIGNAL, EV_TIMEOUT } return true; - } else return false; + } else { + // NOTE: In particular, EINTR needs to cause true to be returned + // here, so that user code has a chance to handle any effects + // of the signal handler before waiting again. + // + // Other errors are very likely to to reoccur for the next + // loop iteration, so there is no value in attempting to + // wait again. + return ret < 0; + } } override void dispose() diff --git a/source/eventcore/drivers/posix/select.d b/source/eventcore/drivers/posix/select.d index dabc491..60a8380 100644 --- a/source/eventcore/drivers/posix/select.d +++ b/source/eventcore/drivers/posix/select.d @@ -65,7 +65,16 @@ final class SelectEventLoop : PosixEventLoop { notify!(EventType.status)(fd); }); return true; - } else return false; + } else { + // NOTE: In particular, EINTR needs to cause true to be returned + // here, so that user code has a chance to handle any effects + // of the signal handler before waiting again. + // + // Other errors are very likely to to reoccur for the next + // loop iteration, so there is no value in attempting to + // wait again. + return ret < 0; + } } override void dispose() From 206d3fa34e932efdcde846cab48fdc6a361b3a2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Thu, 26 Nov 2020 07:54:38 +0100 Subject: [PATCH 2/3] Add a note about signals to EventDriverCore.processEvents. --- source/eventcore/driver.d | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/source/eventcore/driver.d b/source/eventcore/driver.d index 89da18c..00f1916 100644 --- a/source/eventcore/driver.d +++ b/source/eventcore/driver.d @@ -85,6 +85,12 @@ interface EventDriverCore { in the event queue. The function will return after either the specified timeout has elapsed, or once the event queue has been fully emptied. + On implementations that support it, the function will treat + interruptions by POSIX signals as if an event was received and will + cause it to return. However, note that it is generally recommended to + use `EventDriverSignals` instead of raw signal handlers in order to + avoid their pitfalls as far as possible. + Params: timeout = Maximum amount of time to wait for an event. A duration of zero will cause the function to only process pending events. A From 009bf7f5edca80ec95716559cf33f00a31aaf16c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B6nke=20Ludwig?= Date: Thu, 26 Nov 2020 10:45:32 +0100 Subject: [PATCH 3/3] Add return value documentation to doProcessEvents. --- source/eventcore/drivers/posix/driver.d | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source/eventcore/drivers/posix/driver.d b/source/eventcore/drivers/posix/driver.d index e56f6d9..dc2b50d 100644 --- a/source/eventcore/drivers/posix/driver.d +++ b/source/eventcore/drivers/posix/driver.d @@ -336,6 +336,13 @@ package class PosixEventLoop { protected abstract void dispose(); + /** Waits for and processes a single batch of events. + + Returns: + Returns `false` if no event was received before the timeout expired + and `true` if either an event was received, or if the wait was + interrupted by an error or signal. + */ protected abstract bool doProcessEvents(Duration dur); /// Registers the FD for general notification reception.