bpo-36670, regrtest: Fix TestWorkerProcess.stop()

Lib/test/libregrtest/runtest_mp.py

@@ -116,36 +116,41 @@ def __init__(self, worker_id, pending, output, ns, timeout):
         self._popen = None
         self._killed = False
         self._stopped = False
+        # Lock used to make stop() reliable when regrtest is interrupted by
+        # CTRL+c: prevent modifying _popen and _killed attributes at the same
+        # time from two different threads
+        self._lock = threading.RLock()
 
     def __repr__(self):
         info = [f'TestWorkerProcess #{self.worker_id}']
         if self.is_alive():
-            dt = time.monotonic() - self.start_time
-            info.append("running for %s" % format_duration(dt))
+            info.append("running")
         else:
             info.append('stopped')
         test = self.current_test_name
         if test:
             info.append(f'test={test}')
-        popen = self._popen
-        if popen:
-            info.append(f'pid={popen.pid}')
+        with self._lock:
+            if self._popen is not None:
+                dt = time.monotonic() - self.start_time
+                info.append(f'pid={self._popen.pid} ({format_duration(dt)})')
         return '<%s>' % ' '.join(info)
 
     def _kill(self):
-        if self._killed:
-            return
-        self._killed = True
+        with self._lock:
+            popen = self._popen
+            if popen is None:
+                return
 
-        popen = self._popen
-        if popen is None:
-            return
+            if self._killed:
+                return
+            self._killed = True
 
-        print(f"Kill {self}", file=sys.stderr, flush=True)
-        try:
-            popen.kill()
-        except OSError as exc:
-            print_warning(f"Failed to kill {self}: {exc!r}")
+            print(f"Kill {self}", file=sys.stderr, flush=True)
+            try:
+                popen.kill()
+            except OSError as exc:
+                print_warning(f"Failed to kill {self}: {exc!r}")
 
     def stop(self):
         # Method called from a different thread to stop this thread
@@ -177,48 +182,40 @@ def _run_process(self, test_name):
 
         self.current_test_name = test_name
         try:
-            self._killed = False
-            self._popen = run_test_in_subprocess(test_name, self.ns)
-            popen = self._popen
+            popen = run_test_in_subprocess(test_name, self.ns)
+            with self._lock:
+                self._popen = popen
+                self._killed = False
         except:
             self.current_test_name = None
             raise
 
         try:
-            if self._stopped:
-                # If kill() has been called before self._popen is set,
-                # self._popen is still running. Call again kill()
-                # to ensure that the process is killed.
-                self._kill()
-                raise ExitThread
-
             try:
-                stdout, stderr = popen.communicate(timeout=self.timeout)
-            except subprocess.TimeoutExpired:
                 if self._stopped:
-                    # kill() has been called: communicate() fails
-                    # on reading closed stdout/stderr
                     raise ExitThread
 
+                try:
+                    stdout, stderr = popen.communicate(timeout=self.timeout)
+                except Exception:
+                    if self._stopped:
+                        raise ExitThread
+                    else:
+                        raise
+            except subprocess.TimeoutExpired:
                 return self._timedout(test_name)
-            except OSError:
-                if self._stopped:
-                    # kill() has been called: communicate() fails
-                    # on reading closed stdout/stderr
-                    raise ExitThread
-                raise
 
             retcode = popen.returncode
             stdout = stdout.strip()
             stderr = stderr.rstrip()
-
             return (retcode, stdout, stderr)
         except:
             self._kill()
             raise
         finally:
             self._wait_completed()
-            self._popen = None
+            with self._lock:
+                self._popen = None
             self.current_test_name = None
 
     def _runtest(self, test_name):