Skip to content

CHILD HELPERS: use less severe debug level -- sssd-2.9 backport#8433

Merged
alexey-tikhonov merged 1 commit intoSSSD:sssd-2-9from
alexey-tikhonov:concurrent-handlers-2.9
Feb 6, 2026
Merged

CHILD HELPERS: use less severe debug level -- sssd-2.9 backport#8433
alexey-tikhonov merged 1 commit intoSSSD:sssd-2-9from
alexey-tikhonov:concurrent-handlers-2.9

Conversation

@alexey-tikhonov
Copy link
Copy Markdown
Member

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

@alexey-tikhonov alexey-tikhonov added the backport-to-sssd-2-9-4 Corresponds to C8S label Feb 6, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@alexey-tikhonov alexey-tikhonov marked this pull request as ready for review February 6, 2026 11:27
Copy link
Copy Markdown
Contributor

@sumit-bose sumit-bose left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 <sbose@redhat.com>
@sssd-bot
Copy link
Copy Markdown
Contributor

sssd-bot commented Feb 6, 2026

The pull request was accepted by @sumit-bose with the following PR CI status:


🟢 CodeQL (success)
🟢 Build / make-distcheck (success)
🟢 ci / prepare (success)
🟢 ci / system (centos-9) (success)
🟢 Static code analysis / codeql (success)
🟢 Static code analysis / pre-commit (success)
🟢 Static code analysis / python-system-tests (success)


There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging.

@sssd-bot sssd-bot force-pushed the concurrent-handlers-2.9 branch from f455c93 to 96b7a59 Compare February 6, 2026 13:48
@alexey-tikhonov alexey-tikhonov merged commit 4a7245c into SSSD:sssd-2-9 Feb 6, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants