Fix errors with multiprocessing#521
Fix errors with multiprocessing#521
Conversation
jaraco
left a comment
There was a problem hiding this comment.
Looks good. I especially like that the behavior is essentially a no-op except in the affected scenario.
I don't love that the code is detached from its locus of effect, and while the comments help, I'd prefer if we could model the effect in Python syntax, such as a decorator around functools.lru_cache(new) that performs the registration. That decorator could also have a docstring capturing the narrative currently in comments.
The other thing that would be nice to have is a regression test (a test that would replicate the failure with the workaround removed). It might be a little tricky, as the repro steps described in the issue were intermittent, but if it's possible, we should add one that guarantees that multiprocessing usage works in the supported scenarios. Let's try to put together a test to capture the failure. That test should fail on main and pass in this PR.
|
@jaraco Good success catching up on Pull Requests! Do I understand correctly that you are working with Copilot on the requested changes? |
I was hoping to simply assign to Copilot and it would work on it, but it seems that's not the case. I have had success using GitHub Copilot in VS Code and pointing it at issues, so I'll see what it that can achieve (even though it's more manually intensive). |
Before, one could get OSError 22 and BadZipFile errors due to re-used file pointers in forked subprocesses. Fixes python#520
…g the test suite).
Before, one could get OSError 22 and BadZipFile errors due to re-used file pointers in forked subprocesses.
This implements the fix recommended by @anntzer .
Fixes #520