Seldaek / monolog

Sends your logs to files, sockets, inboxes, databases and various web services
https://seldaek.github.io/monolog/
MIT License
20.95k stars 1.9k forks source link

The file does not close after writing #1862

Open sadakov opened 8 months ago

sadakov commented 8 months ago

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 commented 8 months ago

@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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 4 months ago

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.