Skip to content

Avoid Diags lock for syslog output#13398

Open
bneradt wants to merge 1 commit into
apache:masterfrom
bneradt:avoid-diags-lock-for-syslog
Open

Avoid Diags lock for syslog output#13398
bneradt wants to merge 1 commit into
apache:masterfrom
bneradt:avoid-diags-lock-for-syslog

Conversation

@bneradt

@bneradt bneradt commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Syslog-only diagnostics take the same mutex used to serialize file
output, adding avoidable contention when many threads report a shared
failure.

This limits the diagnostics mutex to FILE-backed destinations and
removes the obsolete FreeBSD exception because syslog provides its own
thread safety.

Fixes: #7374

Syslog-only diagnostics take the same mutex used to serialize file
output, adding avoidable contention when many threads report a shared
failure.

This limits the diagnostics mutex to FILE-backed destinations and
removes the obsolete FreeBSD exception because syslog provides its own
thread safety.

Fixes: apache#7374
@bneradt bneradt added this to the 11.0.0 milestone Jul 16, 2026
Copilot AI review requested due to automatic review settings July 16, 2026 21:02
@bneradt bneradt self-assigned this Jul 16, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces diagnostics-related lock contention by avoiding the Diags mutex when the configured destination is syslog-only. This aligns with ATS’s multi-threaded runtime characteristics where high fan-out warnings/notes can otherwise serialize on a shared lock.

Changes:

  • Limits Diags::print_va() locking to FILE-backed outputs (diagslog/stdout/stderr and regression stderr).
  • Removes the FreeBSD-specific “hold lock through syslog” exception so syslog is no longer serialized behind the diagnostics mutex.
  • Minor refactor to use a single output reference for the selected diags_level.

@JosiahWI JosiahWI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are existing race conditions in this part of the diags code I think, but I don't think this change adds any new races beyond what already existed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Avoid locks in DiagsError when target is syslog

3 participants