-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Console] Use a partial buffer in SymfonyStyle #39160
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
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.
The bug exists on lower branches, isn't it?
Can we mark the new class as internal to make this target 4.4?
Thanks, I picked the wrong base branch when creating the PR |
fd29aef
to
3d4249c
Compare
I'm not sure we need to fix this "issue". In the original issue report, the reproducer in #39156 is a code that writes a string 26 millions of times. How is that realistic? |
@javiereguiluz the original laravel issue was abotu a long-running command outputting lots of different things. The minimal reproducer is not meant to be a realistic case, but a minimal case. |
The original Laravel issue report says that this problem occurs "After running it for a long time". How long is "long" considered here? Hours? Days? Weeks? I wouldn't try to fix this because for now it looks like a "non issue". (In PHP you must restart long-running processes from time to time to avoid these "memory leaks" or other issues, right?) |
The script leaks. The fix is not that complexe...
NOooo!!!! Why people believe that having a memory leak is the normal behavior? I hope I won't offend you @javiereguiluz , but I'm really tired of people saying PHP sucks and have a bad memory management. |
If this is not the problem, our script can run forever without memory leaks. This has nothing to do with the length of time the problem occurred. If the business needs it, the trigger point can be reached in a day. If this function is not used, it will never be triggered. What is the significance of this function? Knowing that there is a problem that can be solved, but through a method that is not a solution? |
This PR was merged into the 4.4 branch. Discussion ---------- [Console] Fix console closing tag | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - When using SymfonyStyle, in some cases, closing a tag, is called twice. In the following code `<question>do you want <comment>something</>?</>` the first `</>` close both the `comment` and the `question` tags.  The reason is, part of the content is sent in 2 Outputs (see #39160 for another issue), and both outputs share the same `$styleStack`. This PR updates the `OutputFormatter::__clone` method to prevent sharing the same state. Commits ------- 2834c27 Fix console closing tag
3d4249c
to
18fca29
Compare
Thank you @jderusse. |
Thanks for pinging. Yes, you fix is OK.
|
Yes, the test still proves that the leak is stopped. 👍🏻 |
Symfony style needs to buffer output in order to get the last 2 chars in order to prepend Block.
By using a
BufferedOutput
symfony bufferize everything while it not needed.This PR adds a new
TrimmedBufferOutput
that keep only the N last chars.