[issue39995] test_concurrent_futures: ProcessPoolSpawnExecutorDeadlockTest.test_crash() fails with OSError: [Errno 9] Bad file descriptor

[Thomas Moreau]

Thomas Moreau <thomas.moreau.2010 at gmail.com> added the comment:

Sorry I just saw this. It seems that I introduced this regression.

One of the goal of having a `ThreadWakeup` and not a `SimpleQueue` is to avoid using locks that can hinder the performance of the executor. I don't remember the exact details but I think I did some benchmark and it was giving lower performances for large set of small tasks (not so sure anymore). If a fully synchronous method is chosen, maybe it is safer to rely on a `SimpleQueue` as it will be lower maintenance. If the solution proposed by aeros is chosen, the event can probably be replaced by a lock no? It would be easier to understand I guess.

>From the failures, it seems to be a race condition between `shutdown` and `terminate_broken`. The race condition should not be possible in `submit` as in principle, the `join_executor_internals` is only called when no new task can be submitted.
One solution would be to use the `self._shutdown_lock` from the executor to protect the call to `close` in `terminate_broken` and the call to `self._thread_wakeup.wakeup` in `shutdown`. That way, the lock is only acquired at critical points without being used all the time. This could also be done by adding `lock=True/False` to only lock the potentially dangerous calls.

----------
nosy: +tomMoral

_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue39995>
_______________________________________