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

The file does not close after writing #1862

Closed
sadakov opened this issue Dec 18, 2023 · 6 comments
Closed

The file does not close after writing #1862

sadakov opened this issue Dec 18, 2023 · 6 comments
Labels
Milestone

Comments

@sadakov
Copy link

sadakov commented Dec 18, 2023

Monolog version 3

We use Laravel and queues. Queue is maintained by supervisor.

The queue writes a log. But after deleting the log, the task in the queue no longer writes to the log. All because the file does not close after writing (fclose).

Here src/Monolog/Handler/StreamHandler.php, method write, but then it doesn't close stream.

I tested, if I added a fclose($stream) at the end of the write method, the problem disappears.

@kirkbushell
Copy link

@Seldaek I think this may be a good path forward regarding the broken pipe errors people are seeing (including us, as well - as of 2 months ago). Do you foresee any issues with the stream being closed at the end of each write?

@Rydgel
Copy link

Rydgel commented Jan 24, 2024

We have this issue as well in production, seems like file descriptors aren't getting closed as the issue appears more and more as the server uptime increase. Restarting the kube pod fix the issue for a while. So yeah there must be some missing fclose somewhere.

More information: PHP 8.2, Laravel 10, Monolog 3.5
Using StreamHandler into stderr, for Datadog

Really started to notice random 500 errors caused by this on our app after upgrading to PHP 8.2 and Monolog 3

Edit: we are using php-fpm

@stof
Copy link
Contributor

stof commented Jan 24, 2024

Monolog handlers have a concept of closing them. The StreamHandler will close the stream only when closing the handler (assuming you configured it in a way where the handler is the one opening the stream, not when passing it a stream managed externally that it could not re-open later).
Opening and closing the stream for each log message being written would quickly turn your logger as a performance bottleneck.

@Rydgel
Copy link

Rydgel commented Jan 24, 2024

Yeah, as far as I understand it should closes on destruct. There is most likely a leak somewhere though, can the destruct call get skipped for some reasons? (crash, long running processes, etc)

@Seldaek
Copy link
Owner

Seldaek commented Apr 12, 2024

I think maybe what we could try here is to close the stream when reset() is called, as that should be called between jobs in long running processes. That should ensure we don't keep a resource open for too long, assuming Laravel resets monolog correctly when needed.

@Seldaek Seldaek added this to the 2.x milestone Apr 12, 2024
Seldaek added a commit that referenced this issue Nov 11, 2024
* Close file handle after each write, refs #1862, refs #1860, refs #1634

* Modify patch to retry writes once if they fail, then throw if not

* Fix php7.2 build
@Seldaek Seldaek modified the milestones: 2.x, 3.x Nov 11, 2024
@Seldaek
Copy link
Owner

Seldaek commented Nov 11, 2024

Ok this should be fixed in the next release as long as you call reset() correctly between every job in a long running process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants