distributed/tests/test_asyncprocess.py::test_signal flaky

[fjetter]

https://github.com/dask/distributed/runs/6513746712?check_suite_focus=true

as part of #6381

running into an async test timeout

[crusaderky]

I'm getting a new error:
https://github.com/dask/distributed/pull/6633/checks?check_run_id=7150089143

>       assert proc.exitcode in (-signal.SIGTERM, 255)
E       assert -9 in (-15, 255)
E        +  where -9 = <AsyncProcess SpawnProcess-220>.exitcode

I cannot understand how the process received SIGKILL after we sent SIGTERM 🤷‍♂️

[fjetter]

There are a couple of places we are issuing a SIGKILL

• Worker memory_monitor
• A couple of error handlers in nanny

  distributed/distributed/nanny.py

  Lines 672 to 681 in c992f80

  	logger.exception("Nanny failed to start process", exc_info=True)
  	self.process.terminate()
  	self.status = Status.failed
  	return self.status
  	try:
  	msg = await self._wait_until_connected(uid)
  	except Exception:
  	logger.exception("Failed to connect to process")
  	self.status = Status.failed
  	self.process.terminate()

• Lastly, while shutting down the nanny, if the worker process didn't close in time

  distributed/distributed/nanny.py

  Line 774 in c992f80

  await process.terminate()

The last item on this list reminds me of #6493 (comment) where I was also finding a worker not closing as expected

[fjetter]

Turns out, the AsyncProcess also has a finalizer that is terminating the process. I do not see any logs in the failing CI, though

distributed/distributed/process.py

Lines 363 to 369 in c992f80

	def _asyncprocess_finalizer(proc):
	if proc.is_alive():
	try:
	logger.info(f"reaping stray process {proc}")
	proc.terminate()
	except OSError:
	pass

[crusaderky]

Worker memory_monitor

The nanny isn't involved in this test though

All those calls to terminate() sends SIGTERM, not SIGKILL:

distributed/distributed/process.py

Lines 261 to 288 in c992f80

	def terminate(self) -> asyncio.Future[None]:
	"""Terminate the child process.
	This method returns a future.
	See also
	--------
	multiprocessing.Process.terminate
	"""
	self._check_closed()
	fut: Future[None] = Future()
	self._watch_q.put_nowait({"op": "terminate", "future": fut})
	return fut
	def kill(self) -> asyncio.Future[None]:
	"""Send SIGKILL to the child process.
	On Windows, this is the same as terminate().
	This method returns a future.
	See also
	--------
	multiprocessing.Process.kill
	"""
	self._check_closed()
	fut: Future[None] = Future()
	self._watch_q.put_nowait({"op": "kill", "future": fut})
	return fut

distributed/distributed/process.py

Lines 219 to 224 in c992f80

	elif op == "terminate":
	# Send SIGTERM
	_call_and_set_future(loop, msg["future"], process.terminate)
	elif op == "kill":
	# Send SIGKILL
	_call_and_set_future(loop, msg["future"], process.kill)

[crusaderky]

#6671 xfails the tests on MacOSX when the test expects SIGINT or SIGTERM as a return code but it gets SIGKILL instead.
Should we keep this issue open for a future fix? I am personally disinclined to spend more time on it, as this hardly affects the tested software. Any third-party nanny that tests our exit codes should only ever check zero vs. nonzero.