Make Process.wait safe to retry after timeout

src/pipe.toit

@@ -252,9 +252,11 @@ fork use-path stdin stdout stderr command arguments -> List
 /** The result of forking a process with $fork. */
 class Process:
   fork-data_/List
+  waiter_/ProcessWait_
   exit-value_/int? := null
 
   constructor .fork-data_:
+    waiter_ = ProcessWait_ fork-data_[3]
 
   /** The pid of the child process. */
   pid -> any: return fork-data_[3]
@@ -288,7 +290,7 @@ class Process:
   */
   wait -> int:
     if exit-value_ == null:
-      exit-value_ = wait_ pid
+      exit-value_ = waiter_.wait
     return exit-value_
 
   static wait_ child-process -> int:
@@ -302,7 +304,7 @@ class Process:
   Tells the system that we don't want to wait for the child process to finish.
   */
   wait-ignore -> none:
-    dont-wait-for_ pid
+    waiter_.ignore
 
   /**
   Returns the exit code of the finished process.
@@ -330,6 +332,48 @@ class Process:
     if (value & PROCESS-SIGNALLED_) == 0: return null
     return (value >> PROCESS-SIGNAL-SHIFT_) & PROCESS-SIGNAL-MASK_
 
+monitor ProcessWait_:
+  child-process_/any
+  state_/monitor.ResourceState_? := null
+  exit-value_/int? := null
+  ignored_ := false
+
+  constructor .child-process_:
+    add-finalizer this:: ignore
+
+  wait -> int:
+    if ignored_: throw "PROCESS_WAIT_IGNORED"
+    if exit-value_: return exit-value_
+
+    if state_ == null:
+      // Do not let a timeout land between registering the subprocess and
+      // retaining its ResourceState_. A retry must use the same registration.
+      critical-do --no-respect-deadline:
+        if state_ == null:
+          wait-for_ child-process_
+          state_ = monitor.ResourceState_ process-resource-group_ child-process_
+
+    value := state_.wait
+    // Retain the result and dispose the notifier as one operation. This also
+    // makes concurrent and repeated waits observe the same result.
+    critical-do --no-respect-deadline:
+      if not exit-value_:
+        exit-value_ = value
+        state_.dispose
+        state_ = null
+        remove-finalizer this
+    return exit-value_
+
+  ignore -> none:
+    if ignored_ or exit-value_: return
+    critical-do --no-respect-deadline:
+      if state_:
+        state_.dispose
+        state_ = null
+      dont-wait-for_ child-process_
+      ignored_ = true
+      remove-finalizer this
+
 /**
 Forks a process.
 

tests/pipe2_test.toit

@@ -9,7 +9,7 @@ import host.file
 import host.pipe
 import host.pipe show windows-escape_
 import semver
-import system show platform PLATFORM-FREERTOS
+import system show platform PLATFORM-FREERTOS PLATFORM-WINDOWS
 
 test-exit-value command args expected-exit-value sleep-time/int:
   complete-args := [command] + args
@@ -49,6 +49,41 @@ test-exit-signal sleep-time/int:
   expect-equals null (pipe.exit-code exit-value)
   expect-equals SIGKILL (pipe.exit-signal exit-value)
 
+test-interrupted-wait:
+  // Keep stdin open so the child cannot finish before the waits time out.
+  process := pipe.fork --create-stdin "cat" ["cat"]
+
+  errors := List 3
+  errors.size.repeat:
+    errors[it] = catch: with-timeout --ms=5: process.wait
+
+  SIGKILL := 9
+  pipe.kill_ process.pid SIGKILL
+  exit-value := with-timeout --ms=1_000: process.wait
+
+  errors.do: expect-equals DEADLINE-EXCEEDED-ERROR it
+  expect-equals SIGKILL (pipe.exit-signal exit-value)
+  expect-equals exit-value process.wait
+  expect-equals SIGKILL process.exit-signal
+
+test-ignored-soft-kill:
+  // Wait for the shell to install the handler before sending the signal.
+  process := pipe.fork
+      --create-stdout
+      "sh"
+      ["sh", "-c", "trap '' TERM; echo ready; exec sleep 60"]
+  process.stdout.in.read
+
+  SIGTERM := 15
+  SIGKILL := 9
+  pipe.kill_ process.pid SIGTERM
+  error := catch: with-timeout --ms=20: process.wait
+  pipe.kill_ process.pid SIGKILL
+  exit-value := with-timeout --ms=1_000: process.wait
+
+  expect-equals DEADLINE-EXCEEDED-ERROR error
+  expect-equals SIGKILL (pipe.exit-signal exit-value)
+
 main:
   test-windows-escaping
 
@@ -64,6 +99,9 @@ main:
   test-exit-signal 0
   test-exit-signal 20
 
+  25.repeat: test-interrupted-wait
+  if platform != PLATFORM-WINDOWS: test-ignored-soft-kill
+
 // Tests a private method in pipe.toit.
 test-windows-escaping:
   expect-equals "foo" (windows-escape_ "foo")