Release but don't free CLI streams when executing cli scripts#1906
Merged
Release but don't free CLI streams when executing cli scripts#1906
Conversation
Member
|
Thanks for your research and fix @SpencerMalone. It appears that this code has been updated upstream and may resolve the issue. Could you try applying this patch instead please? php/php-src@0a4a55f#diff-a710e4daa6ff76de76db0782fa9471170421bd585e5bccbaac6faa63be756901L529-L544 |
Contributor
Author
|
Ah yep that more or less does the same thing looking at it, and testing locally it works! Aaaaand I read this logic backwards and thought previously we were leaving them open and now we're closing but we're doing the opposite, so I totally stumbled onto an accidental fix here, but accidentally found the right thing 🤦 . Better lucky than good, I guess? I'll rebase those changes onto this branch. |
dunglas
approved these changes
Oct 8, 2025
alexandre-daubois
approved these changes
Oct 8, 2025
…les to release but not free stdout/in/err. Fixes being unable to log to stdout or error after using frankenphp.ExecutePHPCode
Member
|
Thanks! |
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
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.
I think this closes out #590 if I'm not mistaken.
Here's the diff:
When run on main gets me...
which looks like it exited early (but it didn't)
When run on my branch gets me...
But, in both cases they successfully log to test.log with:
I'm still getting my test env setup enough to write a unit test, but I'm relatively confident this fixes the problem, I'm just not sure if this change has any undesirable side effects.