CHILD HELPERS: use less severe debug level -- sssd-2.9 backport#8433
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly reduces the debug level for a common, non-error condition within the child_sig_handler function. Previously, when multiple SIGCHLD handlers were present, the expected scenario where a handler's waitpid call doesn't find a changed status for its specific child was incorrectly logged as a critical failure. This change appropriately lowers the severity to a trace level and enhances the log message for better clarity. The modification is sound and effectively resolves the issue of excessive and misleading log entries.
sumit-bose
left a comment
sumit-bose
left a comment
There was a problem hiding this comment.
Hi,
thank you for the backport, ACK.
bye,
Sumit
if `child_sig_handler()` is called for unknown pid. If there are N handlers registered and 1 child process exists, all N handlers will be invoked, and N-1 of them will get `waitpid() == 0`. It would be possible to have a single handler registed that would manage a list (or hash table) of `sss_child_ctx`, but it still would have to perform N `waitpid()` calls (`waitpid(-1)` can't be used to avoid handling "foreign" process) so complexity overhead doesn't worth it. Backport of 003c591 Reviewed-by: Sumit Bose <[email protected]>
|
The pull request was accepted by @sumit-bose with the following PR CI status: 🟢 CodeQL (success) There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging. |
f455c93 to
96b7a59
Compare
3 participants
if
child_sig_handler()is called for unknown pid.If there are N handlers registered and 1 child process exists, all N handlers will be invoked, and N-1 of them will get
waitpid() == 0.It would be possible to have a single handler registed that would manage a list (or hash table) of
sss_child_ctx, but it still would have to perform Nwaitpid()calls (waitpid(-1)can't be used to avoid handling "foreign" process) so complexity overhead doesn't worth it.Backport of 003c591