-
Notifications
You must be signed in to change notification settings - Fork 425
fix: don't flush env between requests #1814
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
|
I just now also realized that it's not even necessary to flush globals. Their re-import will flush them anyways. Only Also surprised that noone has so far complained about |
|
Alright, I think I finally figured out a cleaner way to reset globals. Insterestingly, only $FILES will not be flushed on import, all other globals are flushed. I think it might make sense to document that |
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. Is it ready to be merged?
# Conflicts: # frankenphp.c
|
Just merged main, should be good to go if CI is green |
|
|
|
Nvm, rather it looks like the |
|
Should be fine now 👍 , linting failure is unrelated to this PR. |
|
Thank you! Great work, as usual. |
This is an alternative to #1799 that instead doesn't flush the
$_ENVglobal at all. I realized that the 'default' state of globals is just a bunch of 0 bits. I also realized that worker mode currently doesn't support$_REQUEST.cc @withinboredom can you check if this also fixes the segfaults you have been seeing?