tests: mark test that depends on multiprocess as Linux-only#838
tests: mark test that depends on multiprocess as Linux-only#838bhearsum merged 1 commit intotaskcluster:mainfrom
Conversation
7f4d1a7 to
6a308f9
Compare
fb8d0b8 to
9391796
Compare
fork context is availablefork context are both available
src/taskgraph/generator.py
Outdated
| # processing. | ||
| if ( | ||
| platform.system() != "Linux" | ||
| or "fork" not in multiprocessing.get_all_start_methods() |
There was a problem hiding this comment.
I'm still a bit confused. Is fork ever not available on linux?
The commit message / PR title say "fix" but it's not clear what this fixes.
There was a problem hiding this comment.
I'm still a bit confused. Is fork ever not available on linux?
It certainly sounds like it should be; after writing the test I got to thinking that it's probably better to be cautious though? It probably doesn't matter in the real world, at least at this time. (I could be convinced to leave this part alone.)
The commit message / PR title say "fix" but it's not clear what this fixes.
I'll update this.
There was a problem hiding this comment.
Is fork ever not available on linux?
Technically you could make it non available but then the condition wouldn't catch it because python doesn't check if it can fork or not (and honestly, it's probable that python wouldn't even start without fork available on linux).
I got to thinking that it's probably better to be cautious though
I don't think it's worth making the condition more confusing than it need to be for a reader. If I see this without context it's going to make me think that somehow fork could be unavailable and probably overthink it...
FTR python does something like this to populate the start methods (I left the comment because it's interesting for the macos case where you were seeing crashes):
if sys.platform != 'win32':
_concrete_contexts = {
'fork': ForkContext(),
'spawn': SpawnContext(),
'forkserver': ForkServerContext(),
}
# bpo-33725: running arbitrary code after fork() is no longer reliable
# on macOS since macOS 10.14 (Mojave). Use spawn by default instead.
# gh-84559: We changed everyones default to a thread safeish one in 3.14.
if reduction.HAVE_SEND_HANDLE and sys.platform != 'darwin':
_default_context = DefaultContext(_concrete_contexts['forkserver'])
else:
_default_context = DefaultContext(_concrete_contexts['spawn'])
else: # Windows
_concrete_contexts = {
'spawn': SpawnContext(),
}
_default_context = DefaultContext(_concrete_contexts['spawn'])There was a problem hiding this comment.
Alright, I'm convinced!
fork context are both availablefork context are both available
9391796 to
24b2719
Compare
fork context are both availableAlso update the comment around this with more details, so I don't get confused next time...
24b2719 to
cc6a04c
Compare
Also update the comment around this with more details, so I don't get confused next time...