Skip to content

[Fork] Use os.register_at_fork instead of pthread_atfork#32933

Closed
XuanWang-Amos wants to merge 30 commits intogrpc:masterfrom
XuanWang-Amos:PythonFork
Closed

[Fork] Use os.register_at_fork instead of pthread_atfork#32933
XuanWang-Amos wants to merge 30 commits intogrpc:masterfrom
XuanWang-Amos:PythonFork

Conversation

@XuanWang-Amos
Copy link
Copy Markdown
Contributor

No description provided.

@XuanWang-Amos XuanWang-Amos changed the title Use os.register_at_fork instead of pthread_atfork [Fork] Use os.register_at_fork instead of pthread_atfork Apr 24, 2023
@XuanWang-Amos XuanWang-Amos added the release notes: no Indicates if PR should not be in release notes label Apr 24, 2023
XuanWang-Amos added a commit that referenced this pull request Apr 25, 2023
)

Fix: #18075

From comments in #18075, `CPython`
reinitialize the `GIL` after `pthread_atfork` child handler, thus we
shouldn't use any `GIL` related functions in child handler which is what
we're currently doing, this PR uses `os.register_at_fork` to replace
`pthread_atfork` to prevent any undesired bevahior.

This also seems to fixes a thread hanging issue cased by changes in
core: #32869

### Testing:
* Passed existing fork tests. (Note that due to some issues in `Bazel`,
this change was not verified by `Bazel runs_per_test`).
* Tested by patch the core PR, was able to fix Python fork tests:
#32933
XuanWang-Amos added a commit to XuanWang-Amos/grpc that referenced this pull request May 1, 2023
…c#32935)

Fix: grpc#18075

From comments in grpc#18075, `CPython`
reinitialize the `GIL` after `pthread_atfork` child handler, thus we
shouldn't use any `GIL` related functions in child handler which is what
we're currently doing, this PR uses `os.register_at_fork` to replace
`pthread_atfork` to prevent any undesired bevahior.

This also seems to fixes a thread hanging issue cased by changes in
core: grpc#32869

### Testing:
* Passed existing fork tests. (Note that due to some issues in `Bazel`,
this change was not verified by `Bazel runs_per_test`).
* Tested by patch the core PR, was able to fix Python fork tests:
grpc#32933
paulosjca pushed a commit to paulosjca/grpc that referenced this pull request May 4, 2023
…c#32935)

Fix: grpc#18075

From comments in grpc#18075, `CPython`
reinitialize the `GIL` after `pthread_atfork` child handler, thus we
shouldn't use any `GIL` related functions in child handler which is what
we're currently doing, this PR uses `os.register_at_fork` to replace
`pthread_atfork` to prevent any undesired bevahior.

This also seems to fixes a thread hanging issue cased by changes in
core: grpc#32869

### Testing:
* Passed existing fork tests. (Note that due to some issues in `Bazel`,
this change was not verified by `Bazel runs_per_test`).
* Tested by patch the core PR, was able to fix Python fork tests:
grpc#32933
wanlin31 pushed a commit that referenced this pull request May 18, 2023
)

Fix: #18075

From comments in #18075, `CPython`
reinitialize the `GIL` after `pthread_atfork` child handler, thus we
shouldn't use any `GIL` related functions in child handler which is what
we're currently doing, this PR uses `os.register_at_fork` to replace
`pthread_atfork` to prevent any undesired bevahior.

This also seems to fixes a thread hanging issue cased by changes in
core: #32869

### Testing:
* Passed existing fork tests. (Note that due to some issues in `Bazel`,
this change was not verified by `Bazel runs_per_test`).
* Tested by patch the core PR, was able to fix Python fork tests:
#32933
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants