Add main_thread_only execmodel

[zmedico]

In order to prevent tasks from running in a non-main thread, wait for the previous task inside _try_send_to_primary_thread, then schedule the next task. Add a main_thread_only execmodel to distinguish this new behavior from the existing thread execmodel, since users of the thread execmodel expect that tasks can run in multiple threads concurrently. If concurrent remote_exec requests are submitted for the main_thread_only execmodel, then the channel will raise a RemoteError with this message:

concurrent remote_exec would cause deadlock for main_thread_only execmodel

In order for main_thread_only users to avoid this error, remote_exec callers must use the returned channel to wait for a task to complete before they call remote_exec again, as demonstrated in test_assert_main_thread_only. Testing of main_thread_only with pytest-xdist has shown that this behavior is compatible with the existing pytest-xdist usage because it never calls remote_exec more than once per gateway.

Closes: #96

[RonnyPfannschmidt]

How does this behave when multiple tasks are being sent

At first glance it seems like it could break usages

[zmedico]

How does this behave when multiple tasks are being sent

At first glance it seems like it could break usages

I ran the tests localy and it seems to somehow deadlock in this one:
testing/test_gateway.py::TestBasicGateway::test_gateway_status_busy[thread-popen]

It hangs on ch1 = gw.remote_exec("channel.send(1); channel.receive()").

I wonder if there's some way to handle it without a deadlock...

I suppose there's probably some queue.Queue usage that could work here.

[RonnyPfannschmidt]

That test is a small approximate of the case I mentioned

Basically if the main thread is busy and you send in a task, the wait will break the gateway

[RonnyPfannschmidt]

Nope, that just hides the issue somewhere that's hard to control

My rough plan was to completely drop exec models to provide async support for execnet via asyncio/trio/curio

[zmedico]

Nope, that just hides the issue somewhere that's hard to control

Can you help me understand what makes it hard to control? Rather than launch a Thread in _local_schedulexec, I would prefer to use a queue.Queue instance to queue an asynchronous call to self._execpool.spawn here. Would a queue make it any easier to control?

My rough plan was to completely drop exec models to provide async support for execnet via asyncio/trio/curio

That seems like an entirely reasonable thing to do (I noticed #150). However, guaranteed scheduling in the main thread as required to solve pytest-dev/pytest-xdist#620 seems like it might be doable given that in my experience the "thread" execmodel nearly provides this guarantee as it is, but maybe I'm being naive due to my lack of familiarity with execnet in general.

[RonnyPfannschmidt]

@zmedico the key issue here is that it is absolutely valid to run multiple tasks concurrent - so waiting for it to be finished is techically wrong, which is why the test for the rinfo busy fails

unless there is a mechanism to give tagged expectations (for main thread tasks) its unlikel to have it correct

[zmedico]

@zmedico the key issue here is that it is absolutely valid to run multiple tasks concurrent - so waiting for it to be finished is techically wrong, which is why the test for the rinfo busy fails

Thanks for the explanation. So, I suppose the plan would be to add support to tag the expectation when we drop execmodel support.

unless there is a mechanism to give tagged expectations (for main thread tasks) its unlikel to have it correct

In the interim for the elimination of execmodel, I suppose we could add a main_thread execmodel that would only slightly extend the thread execmodel, allowing us to use the main_thread execmodel to tag the expectation.

[RonnyPfannschmidt]

introducing a main_thread_only model could be a good starting point

[zmedico]

@RonnyPfannschmidt This main_thread_only implementation seems to work pretty well for me in combination with a pytest-xdist patch that I've created to utilize it. I tend to see a lot of pytest-xdist worker crashes like worker 'gw0' crashed while running ... for one of my projects (I'm using a pytest-rerunfailures workaroud in gentoo/portage#1273), and with these patches I'm not seeing any crashes right now which I was able to trigger locally before.

[RonnyPfannschmidt]

Unless we find a way to incorporate a timeout or a quick error, the name should indicate its a hack

I'd recommend to add a timeout and erroring when the task can't be started within a reasonable timeframe

There should be a test that validates this error happens after a reasonable timeout

[nicoddemus]

Thanks @zmedico!

Left some minor suggestions to the CHANGELOG. 👍

[nicoddemus]

Thanks @zmedico!

@RonnyPfannschmidt would you like to do the honors of merging/releasing this one? 😁

[RonnyPfannschmidt]

Absolutely, I'll get to it later today

This one is a big plus as it does resolve a issue with xdist paining us ever since exec models where added

[RonnyPfannschmidt]

thanks again !

[zmedico]

Corresponding pytest-xdist MR submitted as pytest-dev/pytest-xdist#1027.

[webknjaz]

This is a possible regression cause: xref pytest-dev/pytest-xdist#1070 (comment)