[GH-ISSUE #1251] restartSysCalls is not defined #517

Closed
opened 2026-03-04 02:15:40 +03:00 by kerem · 4 comments
Owner

Originally created by @gmponos on GitHub (Dec 10, 2018).
Original GitHub issue: https://github.com/Seldaek/monolog/issues/1251

On this line https://github.com/Seldaek/monolog/blob/master/src/Monolog/SignalHandler.php#L96 the restartSyscalls is never defined.. I could not understand what the intention was in order to fix it 😞

Originally created by @gmponos on GitHub (Dec 10, 2018). Original GitHub issue: https://github.com/Seldaek/monolog/issues/1251 On this line https://github.com/Seldaek/monolog/blob/master/src/Monolog/SignalHandler.php#L96 the `restartSyscalls` is never defined.. I could not understand what the intention was in order to fix it 😞
kerem 2026-03-04 02:15:40 +03:00
  • closed this issue
  • added the
    Bug
    label
Author
Owner

@Seldaek commented on GitHub (Dec 11, 2018):

Pretty sure it's a typo and should be $this->signalRestartSyscalls as that one exists and is unused.

<!-- gh-comment-id:446091954 --> @Seldaek commented on GitHub (Dec 11, 2018): Pretty sure it's a typo and should be `$this->signalRestartSyscalls` as that one exists and is unused.
Author
Owner

@Seldaek commented on GitHub (Dec 11, 2018):

Ping @RGustBardon though - also this should be applied on 1.x AFAIK

<!-- gh-comment-id:446092067 --> @Seldaek commented on GitHub (Dec 11, 2018): Ping @RGustBardon though - also this should be applied on 1.x AFAIK
Author
Owner

@RGustBardon commented on GitHub (Dec 11, 2018):

@gmponos: Thank you for reporting the bug! As @Seldaek suggested, it should be $this->signalRestartHandlers.

I could not understand what the intention was in order to fix it

Thank you for attempting to come up with a fix! The idea is as follows:

When registering a signal handler, the user can require calling the signal handler previously registered for this signal after the logging of the signal is done. This is what the $callPrevious parameter of the registerSignalHandler method is responsible for. Previous signal handlers are stored in the previousSignalHandler property.

Another parameter of the registerSignalHandler method ($restartSyscalls) controls whether system calls should be restarted upon a signal (cf. usleep in SignalHandlerTest). These preferences are stored in the signalRestartSyscalls property.

Before PHP 7.1, it was not possible to get the current signal handler. Since PHP 7.1, pcntl_signal_get_handler returns either SIG_DFL (the default system handler for a given signal), SIG_IGN (ignore the signal), or a function (previously registered using pcntl_signal).

If the user wishes to call the previous signal handler after the logging takes place and pcntl_signal_get_handler returns a function, this can be done in a straightforward matter: a call to the previous handler will suffice. This is what happens at the very end of handleSignal. However, in cases the previous signal handler is unknown, handleSignal attempts to call the default signal handler (SIG_DFL) once it is done logging.

Right before making such an attempt, the current signal handler is still the one used for logging. For this reason, the signal handler is temporarily substituted with the default one (the first call to the pcntl_signal function at the end of handleSignal), the process sends the signal to itself (posix_kill), it dispatches the newly installed signal handler (pcntl_signal_dispatch), and finally restores the Monolog signal handler (the second call to the pcntl_signal function).

The current units tests have not been able to identify the problem as they never call registerSignalHandler with both $callPrevious and $restartSyscalls set to true.

@Seldaek: Thank you for bringing the bug to my attention! I have created two pull requests with the same fix (#1252 for the master branch and #1253 for the 1.x branch).

<!-- gh-comment-id:446145042 --> @RGustBardon commented on GitHub (Dec 11, 2018): @gmponos: Thank you for reporting the bug! As @Seldaek suggested, it should be `$this->signalRestartHandlers`. > I could not understand what the intention was in order to fix it Thank you for attempting to come up with a fix! The idea is as follows: When registering a signal handler, the user can require calling the signal handler previously registered for this signal after the logging of the signal is done. This is what the `$callPrevious` parameter of the `registerSignalHandler` method is responsible for. Previous signal handlers are stored in the `previousSignalHandler` property. Another parameter of the `registerSignalHandler` method (`$restartSyscalls`) controls whether system calls should be restarted upon a signal (cf. `usleep` in `SignalHandlerTest`). These preferences are stored in the `signalRestartSyscalls` property. Before PHP 7.1, it was not possible to get the current signal handler. Since PHP 7.1, [`pcntl_signal_get_handler`](https://secure.php.net/manual/en/function.pcntl-signal-get-handler.php) returns either `SIG_DFL` (the default system handler for a given signal), `SIG_IGN` (ignore the signal), or a function (previously registered using [`pcntl_signal`](https://secure.php.net/manual/en/function.pcntl-signal.php)). If the user wishes to call the previous signal handler after the logging takes place and `pcntl_signal_get_handler` returns a function, this can be done in a straightforward matter: a call to the previous handler will suffice. This is what happens at the very end of `handleSignal`. However, in cases the previous signal handler is unknown, `handleSignal` attempts to call the default signal handler (`SIG_DFL`) once it is done logging. Right before making such an attempt, the current signal handler is still the one used for logging. For this reason, the signal handler is temporarily substituted with the default one (the first call to the `pcntl_signal` function at the end of `handleSignal`), the process sends the signal to itself ([`posix_kill`](https://secure.php.net/manual/en/function.posix-kill.php)), it dispatches the newly installed signal handler ([`pcntl_signal_dispatch`](https://secure.php.net/manual/en/function.pcntl-signal-dispatch.php)), and finally restores the Monolog signal handler (the second call to the `pcntl_signal` function). The current units tests have not been able to identify the problem as they never call `registerSignalHandler` with both `$callPrevious` and `$restartSyscalls` set to `true`. @Seldaek: Thank you for bringing the bug to my attention! I have created two pull requests with the same fix (#1252 for the master branch and #1253 for the 1.x branch).
Author
Owner

@Seldaek commented on GitHub (Dec 11, 2018):

Great, thanks for the fixes :)

<!-- gh-comment-id:446146221 --> @Seldaek commented on GitHub (Dec 11, 2018): Great, thanks for the fixes :)
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/monolog#517
No description provided.