GeezerVison is showing github.com — daemon: save the crash status file on builds without libbacktrace by stelfrag · Pull Request #24345 · netdata/netdata
Conversation
stelfrag
commented
Oct 15, 2026
•
edited by coderabbitai
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Summary
#20955 stopped compiling the stacktrace sources on FreeBSD, where libbacktrace does not build, and wrapped their callers in #ifdef HAVE_LIBBACKTRACE. That macro is only set when the bundled libbacktrace is built, which happens on Linux and Windows only. The stacktrace module also has libunwind, execinfo and no-op backends, and those are compiled on every platform except FreeBSD. So on macOS, and on Linux builds without libbacktrace:
- the agent no longer saves the status file when it gets a deadly signal, so the next start reports no exit reason or signal;
- stack trace init and capture are skipped;
-W stacktracetest is gone.
The change:
- sets
HAVE_STACKTRACE in CMake exactly where the stacktrace sources are added, and exports it to config.h like the other libnetdata flags;
- guards the callers with it instead of
HAVE_LIBBACKTRACE;
- makes
stacktracetest skip when the backend cannot capture stack traces.
FreeBSD is unchanged. Packages, static builds and Windows use libbacktrace, so they are unaffected.
Test Plan
- Linux build with
-DENABLE_LIBBACKTRACE=Off (execinfo backend, the same code path as macOS), agent killed with SIGSEGV:
- before: the status file has no exit reason and no signal code;
- after: it records
signal-segmentation-fault, SIGSEGV/SI_USER, and the backtrace backend.
-W stacktracetest passes with the execinfo backend.
Summary by CodeRabbit
- Bug Fixes
- Improved stack-trace support across supported platforms, including signal handling, daemon startup, and build information.
- Stack-trace tests now skip capture checks when capture is unavailable, while still reporting cache-test failures.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
netdata#20955 stopped compiling the stacktrace sources on FreeBSD and guarded
their callers with HAVE_LIBBACKTRACE. That macro is set only when the
bundled libbacktrace is built, so macOS and Linux builds using the
libunwind or execinfo backend lost the crash-time status file save,
stack trace init and capture, and -W stacktracetest.
Set HAVE_STACKTRACE where the stacktrace sources are compiled and guard
the callers with it. FreeBSD is unchanged. stacktracetest now skips
when the backend cannot capture stack traces.
coderabbitai
Bot
commented
Oct 15, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|

No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
-
⚙️ Run configuration
-
- Configuration used: defaults
- Review profile: CHILL
- Plan: Advanced
- Run ID:
03f32d1d-fd46-4162-9991-a77a7b28e1c1
📥 Commits
-
Reviewing files that changed from the base of the PR and between 9cfe18b and 0ed94e4.
📒 Files selected for processing: 10
-
CMakeLists.txt
packaging/cmake/config.cmake.h.in
src/daemon/buildinfo.c
src/daemon/daemon.c
src/daemon/main.c
src/daemon/signal-handler.c
src/daemon/status-file.c
src/libnetdata/CMakeLists.txt
src/libnetdata/log/nd_log-init.c
src/libnetdata/stacktrace/stacktrace-unittest.c
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📝 Walkthrough
-
The build now defines and exports HAVE_STACKTRACE on non-FreeBSD platforms. Stacktrace initialization, daemon handling, reporting, and test entry points use this macro. The stacktrace unit test skips capture tests when capture is unavailable.
Changes: Stacktrace feature guard migration
| Layer / File(s) |
Summary |
Define and export the stacktrace feature flag src/libnetdata/CMakeLists.txt, packaging/cmake/config.cmake.h.in, CMakeLists.txt |
The build sets and exports HAVE_STACKTRACE on non-FreeBSD platforms. The configuration template adds the macro, and the phase-ordering comment lists it. |
Guard runtime stacktrace paths src/libnetdata/log/nd_log-init.c, src/daemon/buildinfo.c, src/daemon/daemon.c, src/daemon/signal-handler.c, src/daemon/status-file.c |
Stacktrace initialization, reporting, fork handling, signal registration, and status-file handling use HAVE_STACKTRACE instead of HAVE_LIBBACKTRACE. |
Guard stacktrace tests and handle unavailable capture src/daemon/main.c, src/libnetdata/stacktrace/stacktrace-unittest.c |
The daemon test entry points use HAVE_STACKTRACE. The unit test skips capture tests when capture is unavailable and returns the cache-collision test result. |
Priority: ➖ Normal
Estimated code review effort: 2 (Simple) | ~12 minutes
Change: Bug fix
Merge Risk: ⚪ Minimal · up to 0ed94
-
This change enables stacktrace paths for supported non-FreeBSD builds. The reviewed build ordering and platform source conditions reveal no material merge-blocking risk.
Security architecture risk: 🔵 Low · up to 0ed94
-
The change restores crash diagnostics on additional builds without adding an inbound interface or bypassing existing crash-report preferences. Risk is low, with additional diagnostic metadata reaching existing storage and reporting paths and some platform-specific lifecycle behavior remaining unverified.
Retained concerns
No architecture-level concerns identified.
Security review details
-
Security Blast Radius
- inferred — The directly affected boundary is the daemon or plugin process and its existing diagnostic sinks on builds newly admitted by HAVE_STACKTRACE. Signal delivery still enters the existing handler; the changed registration line supplies a fixed filtering name rather than a new caller-selected callback or privileged operation.
Trust Boundaries and Controls
- observed — The newly enabled signal path retains the availability and async-signal-safety checks; both execinfo and libunwind reject signal-time capture. Remote reporting separately retains the existing crash-reports configuration gate, startup-save condition, and deduplication checks. The PR does not bypass those controls.
Resilience and Maintainability Implications
- observed — The inherited status writer retries interrupted writes and uses a temporary regular file, fsync, permission application, close, and rename before replacing the status file. Startup reloads persisted status. These mechanisms support partial-write containment; they do not establish power-loss durability of the directory entry.
Pre-merge checks: 4 · 1
-
Failed checks: 1 warning
| Check name |
Status |
Explanation |
Resolution |
| Docstring Coverage |
 |
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 7 files. (3 skipped: … |
Write docstrings for the functions missing them to satisfy the coverage threshold. |
Passed checks: 4 passed
-
| Check name |
Status |
Explanation |
| Description Check |
 |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check |
 |
The title clearly identifies the primary change: enabling crash status-file saving for builds without libbacktrace. It matches the pull request objectives and changeset. |
| Linked Issues check |
 |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check |
 |
Check skipped because no linked issues were found for this pull request. |
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 7 files. (3 skipped: 3 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
❤️ Share
-
Comment @coderabbitai help to get the list of available commands.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cubic-dev-ai
Bot
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All reported issues were addressed across 10 files
Architecture diagram
sequenceDiagram
participant CMake as CMake Build System
participant Config as config.h
participant Daemon as Netdata Daemon
participant Signal as Signal Handler
participant Log as Log System
participant StatusFile as Status File Manager
participant Stacktrace as Stacktrace Module
participant Backend as Stacktrace Backend
Note over CMake,Backend: Build-time Configuration Flow
CMake->>CMake: Check OS platform
alt Non-FreeBSD (Linux, macOS, etc.)
CMake->>CMake: Compile stacktrace sources
CMake->>CMake: Set HAVE_STACKTRACE=True
else FreeBSD
CMake->>CMake: Skip stacktrace sources
CMake->>CMake: HAVE_STACKTRACE not set
end
CMake->>Config: Export HAVE_STACKTRACE macro
Config-->>Daemon: Compile-time define available
Note over Daemon,Backend: Runtime Initialization Flow
Daemon->>Log: Initialize logging system
Log->>Stacktrace: stacktrace_init()
alt HAVE_STACKTRACE defined
Stacktrace->>Backend: Initialize backend
Backend-->>Stacktrace: Backend ready
Stacktrace-->>Log: Init complete
else HAVE_STACKTRACE undefined
Stacktrace-->>Log: Skip init
end
Daemon->>Signal: Initialize signal handlers
Signal->>Stacktrace: Set signal handler name
alt HAVE_STACKTRACE defined
Stacktrace-->>Signal: Handler name registered
end
Note over Daemon,StatusFile: Deadly Signal Flow
Daemon->>Signal: Receive deadly signal (SIGSEGV, etc.)
Signal->>StatusFile: Record deadly signal
StatusFile->>Stacktrace: Check availability
alt HAVE_STACKTRACE defined AND stacktrace_available()
StatusFile->>Stacktrace: Capture stack trace
Stacktrace->>Backend: Get stack trace data
Backend-->>Stacktrace: Raw stack frames
Stacktrace-->>StatusFile: Stack trace content
StatusFile->>StatusFile: Save status file with exit reason + signal code + stack trace
else HAVE_STACKTRACE undefined OR unavailable
StatusFile->>StatusFile: Save status file without stack trace info
end
StatusFile-->>Daemon: Status persisted
Note over Daemon,Stacktrace: Unittest Flow (-W stacktracetest)
Daemon->>Stacktrace: stacktrace_unittest()
Stacktrace->>Stacktrace: Initialize subsystem
Stacktrace->>Backend: Check capture capability
alt Backend cannot capture stack traces
Stacktrace-->>Daemon: Test SKIPPED (return 0)
else Backend can capture
Stacktrace->>Stacktrace: Run cache collision tests
Stacktrace-->>Daemon: Test results
end
Note over Daemon,Backend: Daemonize Flow
Daemon->>Daemon: become_daemon()
alt HAVE_STACKTRACE defined
Daemon->>Stacktrace: stacktrace_flush()
Stacktrace->>Backend: Flush cached data
Backend-->>Stacktrace: Flush complete
Stacktrace-->>Daemon: Ready for child process
end
Loading
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
stacktrace_cache_unittest() does not need a capture backend, so run it on
every build and skip only the capture checks when stack traces cannot be
captured.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything is working as expected on Windows. LGTM!
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Ferroin
pushed a commit
that referenced
this pull request
Nov 4, 2026
…4345)
* daemon: save the crash status file on builds without libbacktrace
#20955 stopped compiling the stacktrace sources on FreeBSD and guarded
their callers with HAVE_LIBBACKTRACE. That macro is set only when the
bundled libbacktrace is built, so macOS and Linux builds using the
libunwind or execinfo backend lost the crash-time status file save,
stack trace init and capture, and -W stacktracetest.
Set HAVE_STACKTRACE where the stacktrace sources are compiled and guard
the callers with it. FreeBSD is unchanged. stacktracetest now skips
when the backend cannot capture stack traces.
* stacktrace: run the cache test before skipping capture without a backend
stacktrace_cache_unittest() does not need a capture backend, so run it on
every build and skip only the capture checks when stack traces cannot be
captured.
(cherry picked from commit afe9281)
Ported: v2.12 has no src/libnetdata/CMakeLists.txt; set HAVE_STACKTRACE next to the stacktrace sources in the root CMakeLists.txt.
Reviewers
vkalintiris
Awaiting requested review from vkalintiris
vkalintiris is a code owner
Ferroin
Awaiting requested review from Ferroin
Ferroin is a code owner
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|