Repository navigation
Use uuid.uuid4() as the key in serialization. - #7893
Merged
Merged
Conversation
As title, this is to avoid fork()+exec() calls made by uuid1(). Fixes numba#7881
2 tasks done
stuartarchibald
commented
Mar 9, 2022
|
|
||
| @linux_only | ||
| @needs_strace | ||
| def test_no_fork_in_compilation(self): |
Contributor
Author
There was a problem hiding this comment.
Note to self. This test probably needs to be run as a subprocess test on the basis that previous execution of something which triggers a stateful call e.g. uuid.uuid1.getnode() is pretty likely.
Forces the no fork on compile test to run in a subprocess to avoid state from already initialised modules.
stuartarchibald
marked this pull request as ready for review
March 10, 2022 13:09
stuartarchibald
added
3 - Ready for Review
Effort - medium
sklam
reviewed
Mar 16, 2022
sklam
left a comment
sklam
left a comment
Member
There was a problem hiding this comment.
Thanks for the patch! uuid4 should be sufficient given its use in cloudpickle and its implementation should never depend on fork (hopefully).
There are a few comments esp. in the strace code regarding the handling of the subprocess
| u = self.__uuid | ||
| if u is None: | ||
| u = str(uuid.uuid1()) | ||
| u = str(uuid.uuid4()) |
Member
There was a problem hiding this comment.
Note: cloudpickle uses uuid4 for tracking unique classes. If it's safe for cloudpickle, it is safe for Numba
sklam
added
4 - Waiting on author
As title.
stuartarchibald
added
4 - Waiting on reviewer
sklam
added
5 - Ready to merge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As title, this is to avoid fork()+exec() calls made by uuid1().
Fixes #7881