8000 ErrorHandler produces OOM error when trying to handle a previous OOM error · Issue #40824 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

ErrorHandler produces OOM error when trying to handle a previous OOM error #40824

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.

Sign up for GitHub

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

Jump to bottom
Closed
rasmustnilsson opened this issue Apr 15, 2021 · 5 comments
Closed

ErrorHandler produces OOM error when trying to handle a previous OOM error #40824

rasmustnilsson opened this issue Apr 15, 2021 · 5 comments
Labels
Bug ErrorHandler Status: Needs Review

Comments

@rasmustnilsson
Copy link

Symfony version(s) affected: 4.4.16, 5.2.6

Description
On some OOM errors the Symfony ErrorHandler itself throws an OOM error trying to handle the previous error. This is a problem as it can make handling OOM errors impossible to handle.

This is the php output:

Fatal error: Allowed memory size of 33554432 bytes exhausted (tried to allocate 20480 bytes) in /app/src/BugCommand.php on line 18
Fatal error: Allowed memory size of 33554432 bytes exhausted (tried to allocate 32768 bytes) in /app/vendor/symfony/error-handler/Error/OutOfMemoryError.php on line 1

The second fatal error is not expected.

How to reproduce
https://github.com/tachnik/reproduce-oom-in-error-handler-bug
@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@carsonbot
Copy link

Just a quick reminder to make a comment on this. If I don't hear anything I'll close this.

@nicolas-grekas
Copy link
Member

Thanks for providing a reproducer!
It be great if someone could have a closer look.
Or maybe you'd like to investigate @rasmustnilsson?

@carsonbot carsonbot removed the Stalled label Nov 5, 2021
@rasmustnilsson
Copy link
Author

Have you managed to find a workaround?

No, I never found a workaround.

Or maybe you'd like to investigate @rasmustnilsson?

Sorry, I don't have the time.

@sakalys sakalys mentioned this issue Nov 29, 2021
Merged
@sakalys
Copy link
Contributor
sakalys commented Nov 29, 2021

I created a PR (see above), but I don't think that the code is going to be enough. Any feedback or suggestions are welcome.

@fabpot fabpot closed this as completed Nov 29, 2021
fabpot added a commit that referenced this issue Nov 29, 2021
@fabpot
bug #44327 [Debug][ErrorHandler] Increased the reserved memory from 1…
cacce1c 
…0k to 32k (sakalys)

This PR was merged into the 4.4 branch.

Discussion
----------

[Debug][ErrorHandler] Increased the reserved memory from 10k to 32k

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #40824
| License       | MIT
| Doc PR        | n/a

The ErrorHandler's job includes handling out of memory (OOM) exceptions,
therefore it is important that that feature works. In the current state,
when handling OOM exceptions, the error handler produces an OOM error
itself, because the old 10kB reserve is apparently not enough anymore.
To mitigate that, the reserved memory gets bumped to 32k (which is enough).

*Note* I'm not familiar with the whole open source submitting process, so any feedback and instructions on how to improve this PR are welcome.
I am not sure on how to write a unit test to test something like that (talking about the issue here #40824).

Commits
-------

cbac313 Increased the reserved memory from 10k to 32k
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug ErrorHandler Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @nicolas-grekas @xabbuh @sakalys @carsonbot @rasmustnilsson
0