-
Notifications
You must be signed in to change notification settings - Fork 420
fix: concurrent env access #1409
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
Conversation
|
To reiterate , this fixes:
Important to mention:
|
dunglas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Very good work.
withinboredom
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. It might also be worth updating the documentation on worker mode in this PR to inform users how worker mode affects the environment (or doesn't).
|
Thank you! Great as usual. |
This is an attempt at fixing #1395 #1061 #1353
The idea is basically to sandbox the environment on a per-thread basis and to not flush
$_ENVvalues in worker mode.This still allows
putenvto affect the actual environment (in case of forking). It does not fully resolve all environment races in ZTS, but will resolve all races for$_ENV,$_SERVERand throughgetenv().