From 4f9e9c4e94d86b8667bbcb6192ef388e85671318 Mon Sep 17 00:00:00 2001
From: Duncan Coutts <duncan@well-typed.com>
Date: Mon, 9 Jan 2023 00:43:55 +0000
Subject: [PATCH] Move awaitEvent into a proper IOManager API

and have the scheduler use it.

Previously the scheduler calls awaitEvent directly, and awaitEvent is
implemented directly in the RTS I/O managers (select, win32). This
relies on the old scheme where there's a single active I/O manager for
each platform and RTS way.

We want to move that to go via an API in IOManager.{h,c} which can then
call out to the active I/O manager.

Also take the opportunity to split awaitEvent into two. The existing
awaitEvent has a bool wait parameter, to say if the call should be
blocking or non-blocking. We split this into two separate functions:
pollCompletedTimeoutsOrIO and awaitCompletedTimeoutsOrIO. We split them
for a few reasons: they have different post-conditions (specifically the
await version is supposed to guarantee that there are threads runnable
when it completes). Secondly, it is also anticipated that in future I/O
managers the implementations of the two cases will be simpler if they
are separated.
---
 rts/IOManager.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
 rts/IOManager.h | 23 +++++++++++++++++++++
 rts/Schedule.c  | 27 +++++++++++++++---------
 3 files changed, 95 insertions(+), 10 deletions(-)

diff --git a/rts/IOManager.c b/rts/IOManager.c
index f9fe4c2eecfd..377ba90bb44f 100644
--- a/rts/IOManager.c
+++ b/rts/IOManager.c
@@ -546,6 +546,61 @@ bool anyPendingTimeoutsOrIO(CapIOManager *iomgr)
 }
 
 
+void pollCompletedTimeoutsOrIO(Capability *cap)
+{
+    debugTrace(DEBUG_iomanager, "polling for completed IO or timeouts");
+    switch (iomgr_type) {
+#if defined(IOMGR_ENABLED_SELECT)
+        case IO_MANAGER_SELECT:
+          awaitEvent(cap, false);
+          break;
+#endif
+
+#if defined(IOMGR_ENABLED_WIN32_LEGACY) || \
+   (defined(IOMGR_ENABLED_WINIO) && !defined(THREADED_RTS))
+#if defined(IOMGR_ENABLED_WIN32_LEGACY)
+        case IO_MANAGER_WIN32_LEGACY:
+#endif
+#if defined(IOMGR_ENABLED_WINIO)
+        case IO_MANAGER_WINIO:
+#endif
+          awaitEvent(cap, false);
+          break;
+#endif
+        default:
+            barf("pollCompletedTimeoutsOrIO not implemented");
+    }
+}
+
+
+void awaitCompletedTimeoutsOrIO(Capability *cap)
+{
+    debugTrace(DEBUG_iomanager, "waiting for completed IO or timeouts");
+    switch (iomgr_type) {
+#if defined(IOMGR_ENABLED_SELECT)
+        case IO_MANAGER_SELECT:
+          awaitEvent(cap, true);
+          break;
+#endif
+
+#if defined(IOMGR_ENABLED_WIN32_LEGACY) || \
+   (defined(IOMGR_ENABLED_WINIO) && !defined(THREADED_RTS))
+#if defined(IOMGR_ENABLED_WIN32_LEGACY)
+        case IO_MANAGER_WIN32_LEGACY:
+#endif
+#if defined(IOMGR_ENABLED_WINIO)
+        case IO_MANAGER_WINIO:
+#endif
+          awaitEvent(cap, true);
+          break;
+#endif
+        default:
+            barf("pollCompletedTimeoutsOrIO not implemented");
+    }
+    ASSERT(!emptyRunQueue(cap) || getSchedState() != SCHED_RUNNING);
+}
+
+
 void syncIOWaitReady(Capability   *cap,
                      StgTSO       *tso,
                      IOReadOrWrite rw,
diff --git a/rts/IOManager.h b/rts/IOManager.h
index 90e286959320..1be8c1f307d1 100644
--- a/rts/IOManager.h
+++ b/rts/IOManager.h
@@ -307,6 +307,29 @@ void appendToIOBlockedQueue(Capability *cap, StgTSO *tso);
  */
 bool anyPendingTimeoutsOrIO(CapIOManager *iomgr);
 
+/* If there are any completed I/O operations or expired timers, process the
+ * completions as appropriate (which will typically unblock some waiting
+ * threads, but no guarantee). If there are none, return without waiting.
+ *
+ * Called from schedule() both *before* and *after* scheduleDetectDeadlock().
+ */
+void pollCompletedTimeoutsOrIO(Capability *cap);
+
+ /* If there are any completed I/O operations or expired timers, process the
+ * completions as appropriate. If there are none, wait until I/O or a timer
+ * does complete (or we get a signal with a handler) and process the
+ * completions as appropriate.
+ *
+ * Upon return this guarantees that the scheduler run queue is non-empty or
+ * that the scheduler is no longer in the running state. Succinctly, the
+ * post-condition is (!emptyRunQueue(cap) || getSchedState() != SCHED_RUNNING).
+ *
+ * This is only expected to be called if anyPendingTimeoutsOrIO() returns true,
+ * i.e. there actually is something to wait for.
+ *
+ * Called from schedule() both *before* and *after* scheduleDetectDeadlock().
+ */
+void awaitCompletedTimeoutsOrIO(Capability *cap);
 
 #if !defined(THREADED_RTS)
 /* Check whether there is any completed I/O or expired timers. If so,
diff --git a/rts/Schedule.c b/rts/Schedule.c
index 4f0dcf3e81ce..7aac5047b474 100644
--- a/rts/Schedule.c
+++ b/rts/Schedule.c
@@ -303,7 +303,7 @@ schedule (Capability *initialCapability, Task *task)
     // this point when blocked on an IO Port.  If this is the case the only
     // thing that could unblock it is an I/O event.
     //
-    // win32: might be here due to awaitEvent() being abandoned
+    // win32: might be here due to awaitCompletedTimeoutsOrIO() being abandoned
     // as a result of a console event having been delivered or as a result of
     // waiting on an async I/O to complete with WinIO.
 
@@ -319,10 +319,10 @@ schedule (Capability *initialCapability, Task *task)
         /* Notify the I/O manager that we have nothing to do.  If there are
            any outstanding I/O requests we'll block here.  If there are not
            then this is a user error and we will abort soon.  */
-        /* TODO: see if we can rationalise these two awaitEvent calls before
-         *       and after scheduleDetectDeadlock().
+        /* TODO: see if we can rationalise these two awaitCompletedTimeoutsOrIO
+         *       calls before and after scheduleDetectDeadlock().
          */
-        awaitEvent (cap, emptyRunQueue(cap));
+        awaitCompletedTimeoutsOrIO(cap);
 #else
         ASSERT(getSchedState() >= SCHED_INTERRUPTING);
 #endif
@@ -913,8 +913,8 @@ scheduleCheckBlockedThreads(Capability *cap USED_IF_NOT_THREADS)
      * If the run queue is empty, and there are no other threads running, we
      * can wait indefinitely for something to happen.
      *
-     * TODO: see if we can rationalise these two awaitEvent calls before
-     * and after scheduleDetectDeadlock()
+     * TODO: see if we can rationalise these two awaitCompletedTimeoutsOrIO
+     * calls before and after scheduleDetectDeadlock()
      *
      * TODO: this test anyPendingTimeoutsOrIO does not have a proper
      * implementation the WinIO I/O manager!
@@ -927,13 +927,20 @@ scheduleCheckBlockedThreads(Capability *cap USED_IF_NOT_THREADS)
      * The WinIO I/O manager does not use either the sleeping_queue or the
      * blocked_queue, but it's implementation of anyPendingTimeoutsOrIO still
      * checks both! Since both queues will _always_ be empty then it will
-     * _always_ return false and so awaitEvent will _never_ be called here for
-     * WinIO. This may explain why there is a second call to awaitEvent below
-     * for the case of !defined(THREADED_RTS) && defined(mingw32_HOST_OS).
+     * _always_ return false and so awaitCompletedTimeoutsOrIO will _never_ be
+     * called here for WinIO. This may explain why there is a second call to
+     * awaitCompletedTimeoutsOrIO below for the case of !defined(THREADED_RTS)
+     * && defined(mingw32_HOST_OS).
      */
     if (anyPendingTimeoutsOrIO(cap->iomgr))
     {
-        awaitEvent (cap, emptyRunQueue(cap));
+        if (emptyRunQueue(cap)) {
+            // block and wait
+            awaitCompletedTimeoutsOrIO(cap);
+        } else {
+            // poll but do not wait
+            pollCompletedTimeoutsOrIO(cap);
+        }
     }
 #endif
 }
-- 
GitLab