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 GH-11104: STDIN/STDOUT/STDERR is not available for CLI without a script #11169

Closed
wants to merge 1 commit into from

Conversation

nielsdos
Copy link
Member

I found no reason why this is done this way.
Of course this will allow users to do stupid stuff like fclose(STDOUT); etc. but if they type in that code they clearly know what they're doing...

Easy fix, but I targeted master since this behaviour is long-standing and I'm not even sure this qualifies as a bug or as a feature.
Requesting review from Ilija since he also interacted with the issue.

… a script

I found no reason why this is done this way.
Of course this will allow users to do stupid stuff like
`fclose(STDOUT);` etc. but if they type in that code they clearly know
what they're doing...
Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me neither, definitely seems fine for master 🙂

@nielsdos nielsdos closed this in f6c0c60 May 3, 2023
@kocsismate
Copy link
Member

Why did this commit made the performance of many workloads much worse?

@nielsdos
Copy link
Member Author

nielsdos commented May 5, 2023

@kocsismate this one didn't. See 4ca8daf#comments
The performance impact is only for when php is run in Valgrind (which doesn't happen anyway on real workloads)

@sebastianbergmann
Copy link
Contributor

This causes a regression in PHPUnit (sebastianbergmann/phpunit#5356).

sebastianbergmann added a commit to sebastianbergmann/phpunit that referenced this pull request Sep 16, 2023
…s to communicate their result to the parent process

As of PHP 8.3, specifically as of php/php-src#11169,
it is no longer possible to capture direct writes to standard output.

Such direct writes to standard output interfere with the previous approach
where the child process would print its result to standard output from where
the parent process would read it.

Co-authored-by: Sebastian Bergmann <sb@sebastian-bergmann.de>
Co-authored-by: Arne Blankerts <Arne@Blankerts.de>
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.

4 participants