pytest-xdist breaks asyncio-based code expecting to be run in the main thread

[webknjaz]

When asyncio-based software needs to register signal handlers, calling add_signal_handler() only works when the current thread is main and raises a RuntimeError otherwise.

When I first tried integrating pytest-xdist into aiohttp's test suite a few years ago, we've faced a problem that about 15% of pytest invocations would hit this and had to disable the plugin:

• Optimize tests aio-libs/aiohttp#3419
• pytest-aiohttp + pytest-xdist cause RuntimeError: set_wakeup_fd only works in main thread aio-libs/aiohttp#3450

The reproducer is to clone aio-libs/aiohttp, add -n auto to addopts somewhere in setup.cfg, and re-run make test until you see the traceback. On aiohttp side, it's coming from the watcher.attach_loop() call invoked from the pytest plugin @ https://github.com/aio-libs/aiohttp/blob/742a8b6/aiohttp/pytest_plugin.py#L161.

Now that I've had some time to try to debug what's happening, I've stuck a debugger right before the line that raises RuntimeError and confirmed that the thread wasn't main:

>>> import threading
>>> threading.enumerate()
[<_MainThread(MainThread, started 140666216200000)>, <_DummyThread(Dummy-1, started daemon 140666196751936)>]
>>> threading.current_thread()
<_DummyThread(Dummy-1, started daemon                 
140666196751936)>

@asvetlov It should be possible to fix this on the asyncio side with ThreadedChildWatcher (https://bugs.python.org/issue35621 / python/cpython#14344) but it only appeared in the stdlib since Python 3.8: https://stackoverflow.com/a/58614689/595220.

It's hard to pinpoint where that thread is coming from but it appears to be execnet's fault:

pytest-xdist/src/xdist/workermanage.py

Line 41 in c5fadcd

self.group = execnet.Group()

+ https://github.com/pytest-dev/execnet/blob/116637ab183bc078647f281b52e662650e8178e7/execnet/multi.py#L27.

@nicoddemus do you have any insight into this? I'm rather lost at this point. Is there any way to avoid this race condition?

[RonnyPfannschmidt]

This is a issue that was Int to execnet with the addition of thread models for gevent /eventlet.

A workaround is to downgrade execnet.

Execnet currently is unmaintained.

[webknjaz]

I tried looking into it and thought that maybe using hasprimary=True would help (https://github.com/pytest-dev/execnet/blob/116637a/execnet/gateway_base.py#L165) but it just gets stuck on worker allocation...

@RonnyPfannschmidt is there a known execnet version I need to downgrade to?

[webknjaz]

Tried execnet<1.2 but that's too old for modern Pythons and it explodes trying to call deprecated isAlive().

Sounds like we'll need a backport of ThreadedChildWatcher after all.

@RonnyPfannschmidt do you think there's any reasonable way to have a fixture forcing xdist to bypass scheduling a specific test in workers and just execute it in the main process?

[RonnyPfannschmidt]

When using xdist, no test run in the main process

[graingert]

arn't tests in xdist workers also run in only the main thread?

[RonnyPfannschmidt]

So there is no reasonable way

[graingert]

I've stuck a debugger right before the line that raises RuntimeError and confirmed that the thread wasn't main:

@webknjaz which process was running the breakpoint()? the xdist leader or an xdist worker?

you should be able to track the stack of what created the <_DummyThread(Dummy-1, started daemon 140666196751936)> with tracemalloc

[RonnyPfannschmidt]

@graingert an addition to execnet broke that assumption

[graingert]

@RonnyPfannschmidt do you have a link to the commit that introduced it?

[RonnyPfannschmidt]

Not right now, I'm on mobile atm, and no longer a maintainer for execnet

[webknjaz]

@graingert can't do that right now but I think the thread is spawned by this call https://github.com/pytest-dev/execnet/blob/116637ab183bc078647f281b52e662650e8178e7/execnet/gateway_base.py#L315

[graingert]

so it looks like pytest-xdist will run tests on the main thread almost all the time, it's only if there are concurrent remote_exec calls that execnet will actually spawn a thread, eg we always want to hit: https://github.com/pytest-dev/execnet/blob/116637ab183bc078647f281b52e662650e8178e7/execnet/gateway_base.py#L299-L302

so I had a look around for places that remote_exec is called without an associated wait_close, and only found:

diff --git a/execnet/gateway.py b/execnet/gateway.py
index 8e2c1de..8a0da92 100644
--- a/execnet/gateway.py
+++ b/execnet/gateway.py
@@ -75,7 +75,10 @@ class Gateway(gateway_base.BaseGateway):
         """ return some sys/env information from remote. """
         if update or not hasattr(self, "_cache_rinfo"):
             ch = self.remote_exec(rinfo_source)
-            self._cache_rinfo = RInfo(ch.receive())
+            try:
+                self._cache_rinfo = RInfo(ch.receive())
+            finally:
+                ch.wait_close()
         return self._cache_rinfo
 
     def hasreceiver(self):
-- 
2.30.0

maybe this patch will help?

[webknjaz]

Maybe, to verify that we'd have to run a lot of parallel pytest invocations with the "is main thread" check. I'll try to do this when I have a minute.

[graingert]

it looks like fixing this test will fix the problem properly: https://github.com/pytest-dev/execnet/blob/116637ab183bc078647f281b52e662650e8178e7/testing/test_threadpool.py#L77-L103

[webknjaz]

maybe this patch will help?

@graingert
This patch breaks everything:

INTERNALERROR> AttributeError: 'Channel' object has no attribute 'wait_close'

[graingert]

@webknjaz sorry that's a typo it's waitclose()

diff --git a/execnet/gateway.py b/execnet/gateway.py
index 8e2c1de..8a0da92 100644
--- a/execnet/gateway.py
+++ b/execnet/gateway.py
@@ -75,7 +75,10 @@ class Gateway(gateway_base.BaseGateway):
         """ return some sys/env information from remote. """
         if update or not hasattr(self, "_cache_rinfo"):
             ch = self.remote_exec(rinfo_source)
-            self._cache_rinfo = RInfo(ch.receive())
+            try:
+                self._cache_rinfo = RInfo(ch.receive())
+            finally:
+                ch.waitclose()
         return self._cache_rinfo
 
     def hasreceiver(self):
-- 
2.30.0

[webknjaz]

@graingert that last patch seems to fix this.

[graingert]

@webknjaz I don't think it's quite right, because there's a very small window between when the channel is closed and the primary_thread_task_ready semaphore is set

but it should improve things a lot

[graingert]

Related to pytest-dev/execnet#96

[mattgodbolt]

Hey folks. Apologies, this is a cross-post from the execnet issue, but here for visibility:

wondered if this issue might explain a super-intermittant issue we see.

We're using xdist 1.32, and execnet 1.7.1.

In async tests super rarely we get errors saying RuntimeError: There is no current event loop in thread 'Dummy-1' - which I've traced back to the tests not running on the main thread (and thus not having a default event loop). It seems like this is the same issue - a rare case where tests get run on the "wrong" thread.

Would you concur, and do you have any advice for how to work around it? Thanks in advance.

[zmedico]

I have a proposed solution for this issue in pytest-dev/execnet#243 and I'm getting positive results when I patch pytest-xdist to use it.

[nicoddemus]

This will be available in the next release. 👍

[webknjaz]

FTR the new release of pytest-xdist 3.6.0 explodes in a project that does not use async: #1070 (comment). It seems like it brings some compatibility issues with pytest itself or pytest-cov (which I also see being mentioned in the traceback)...

[zmedico]

FTR the new release of pytest-xdist 3.6.0 explodes in a project that does not use async: #1070 (comment). It seems like it brings some compatibility issues with pytest itself or pytest-cov (which I also see being mentioned in the traceback)...

This looks like a pretty small issue. In pytest-cov it calls node.gateway._rinfo() and this method triggers a deadlock error for the main_thread_only execmodel when the gateway is already busy with another remote_exec call, since _rinfo calls remote_exec here:

https://github.com/pytest-dev/execnet/blob/v2.1.1/src/execnet/gateway.py#L85

The Gateway _rinfo method can be implemented in a way that does not acquire the main thread.

In pytest-dev/execnet#274 I've fixed the Gateway __init__ method to call _rinfo in order to cache the result for later, before the other remote_exec call has started.