Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix two possible issues with the environment escape communication #1555

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

romain-intel
Copy link
Contributor

The first issue is when subprocesses are created after the main process has activated the environment escape. In this case, multiple processes could try to communicate wit the single escaped server which is a big no no. We explicitly block this from happening now. The case we ran into was an object allocated in a parent being gc in a subprocess.

The second issue relates to the GC which could trigger while another communication was happening. This could result in a deadlock since the lock protecting the communication is not reentrant (on purpose). We solve this by preventing the GC from being active during that time.

The first issue is when subprocesses are created after the main process has
activated the environment escape. In this case, multiple processes could try
to communicate wit the single escaped server which is a big no no. We explicitly
block this from happening now. The case we ran into was an object allocated in a parent
being gc in a subprocess.

The second issue relates to the GC which could trigger *while* another communication
was happening. This could result in a deadlock since the lock protecting the communication
is not reentrant (on purpose). We solve this by preventing the GC from being active during
that time.
@savingoyal savingoyal merged commit f8490c2 into master Sep 27, 2023
19 of 20 checks passed
@savingoyal savingoyal deleted the fix/escape-hatch-deadlocks branch September 27, 2023 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants