| msg226805 - (view) |
Author: Dan O'Reilly (dan.oreilly) * |
Date: 2014-09-11 22:33 |
| This is essentially a dupe of , but it was suggested I open a new issue, since that one ended up being used to fix this same problem in concurrent.futures, and was subsequently closed. Right now, should a worker process in a Pool unexpectedly get terminated while a blocking Pool method is running (e.g. apply, map), the method will hang forever. This isn't a normal occurrence, but it does occasionally happen (either because someone sends a SIGTERM, or because of a bug in the interpreter or a C-extension). It would be preferable for multiprocessing to follow the lead of concurrent.futures.ProcessPoolExecutor when this happens, and abort all running tasks and close down the Pool. Attached is a patch that implements this behavior. Should a process in a Pool unexpectedly exit (meaning, *not* because of hitting the maxtasksperchild limit), the Pool will be closed/terminated and all cached/running tasks will raise a BrokenProcessPool exception. These changes also prevent the Pool from going into a bad state if the "initializer" function raises an exception (previously, the pool would end up infinitely starting new processes, which would immediately die because of the exception). One concern with the patch: The way timings are altered with these changes, the Pool seems to be particularly susceptible to in certain cases. If processes in the Pool are being restarted due to maxtasksperchild just as the worker is being closed or joined, there is a chance the worker will be forked while some of the debug logging inside of Pool is running (and holding locks on either sys.stdout or sys.stderr). When this happens, the worker deadlocks on startup, which will hang the whole program. I believe the current implementation is susceptible to this as well, but I could reproduce it much more consistently with this patch. I think its rare enough in practice that it shouldn't prevent the patch from being accepted, but thought I should point it out. (I do think should be addressed, or at the very least internal I/O locks should always reset after forking.) |
|
|
| msg294968 - (view) |
Author: Francis Bolduc (Francis Bolduc) |
Date: 2017-06-01 20:53 |
| This problem also happens simply by calling sys.exit from one of the child processes. The following script exhibits the problem: import multiprocessing import sys def test(value): if value: sys.exit(123) if __name__ == '__main__': pool = multiprocessing.Pool(4) cases = [0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0] pool.map(test, cases) |
|
|
| msg315684 - (view) |
Author: Oscar Esteban (oesteban) * |
Date: 2018-04-24 02:18 |
| We use multiprocessing to parallelize many tasks that run either python code or call subprocess.run that are memory hungry. At times the OOM Killer kicks in. When one of the workers is killed, the queue never returns an error for the task being run by the worker. Are there any plans to merge the patch proposed in this issue? |
|
|
| msg315687 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2018-04-24 05:47 |
| Oscar, the patch posted here needs updating for the latest git master. If you want to avoid this issue, you can also use concurrent.futures where the issue is fixed. |
|
|
| msg329381 - (view) |
Author: Oscar Esteban (oesteban) * |
Date: 2018-11-06 20:02 |
| Hi Antoine, I may take a stab at it. Before I start, should I branch from master or from 3.7.1 (as 3.7 is still accepting bugfixes). Best, Oscar |
|
|
| msg329383 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2018-11-06 20:28 |
| You should start from master. Bugfixes can backported afterwards if appropriate. Thanks! |
|
|
| msg329770 - (view) |
Author: Oscar Esteban (oesteban) * |
Date: 2018-11-12 22:57 |
| I tried to reuse as much as I could from the patch, but it didn't solve the issue at first. I have changed the responsibility of identifying and prescribing a solution when a worker got killed. In the proposed patch, the thread handling results (i.e. tasks queued by one worker as done) was responsible. In the PR, the responsibility is reassigned to the thread handling workers (since, basically, one or more workers suddenly die). The patch defined a new BROKEN state that was assigned to the results handler thread. I transferred this behavior to the worker handler thread. But, I'm guessing that the BROKEN state should be assigned to the Pool object instead, to be fully semantic. Although that would require passing the reference to the object around and complicate unnecessarily the implementation. Happy to reconsider though. I added three tests, one that was present with the patch, a variation of it adding some wait before killing the worker, and the one that Francis Bolduc posted here (https://bugs.python.org/issue22393#msg294968). Please let me know whether any conversation about this bug should take place in GitHub, with the PR instead of here. Thanks a lot for the guidance, Antoine. |
|
|
| msg333895 - (view) |
Author: Chris Markiewicz (cjmarkie) * |
Date: 2019-01-17 19:39 |
| Just a bump to note that the PR (10441) is ready for another round of review. |
|
|
| msg351754 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2019-09-10 22:50 |
| I just marked bpo-38084 as duplicate of this issue. I manually merged the nosy lists. |
|
|
| msg390775 - (view) |
Author: Marko (kormang) |
Date: 2021-04-11 11:57 |
| I've created . I think it would be better to have universal solution. And not specific ones, like in . Haven't checked the PRs, though. |
|
|
| msg390780 - (view) |
Author: Marko (kormang) |
Date: 2021-04-11 12:18 |
| Somewhat related with asyncio.StreamReader |
|
|