GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — daemon: save the crash status file on builds without libbacktrace by stelfrag · Pull Request #24345 · netdata/netdata

Conversation

@stelfrag

@stelfrag stelfrag commented Oct 15, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator
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.

All reactions
@stelfrag
daemon: save the crash status file on builds without libbacktrace
4fb6436
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

coderabbitai Bot commented Oct 15, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning 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 Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed 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 Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed 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)
  • Create a new PR
  • 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
  • X
  • Mastodon
  • Reddit
  • LinkedIn

Comment @coderabbitai help to get the list of available commands.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot added area/packaging Packaging and operating systems support area/daemon area/build Build system (autotools and cmake). labels Oct 15, 2026
cubic-dev-ai[bot]
cubic-dev-ai Bot reviewed Oct 15, 2026
View reviewed changes

@cubic-dev-ai cubic-dev-ai Bot left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

All reactions
Comment thread src/libnetdata/stacktrace/stacktrace-unittest.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread src/daemon/status-file.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@stelfrag
stacktrace: run the cache test before skipping capture without a backend
0ed94e4
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.
@sonarqubecloud

sonarqubecloud Bot commented Oct 15, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@stelfrag
stelfrag marked this pull request as ready for review October 16, 2026 15:53
@stelfrag
stelfrag requested review from a team, Ferroin, thiagoftsm and vkalintiris as code owners October 16, 2026 15:53
thiagoftsm
thiagoftsm approved these changes Oct 16, 2026
View reviewed changes

@thiagoftsm thiagoftsm left a comment

Copy link
Copy Markdown
Contributor

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.

All reactions
@stelfrag
stelfrag merged commit afe9281 into netdata:master Oct 16, 2026
207 of 208 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@stelfrag
stelfrag deleted the fix-status-file-no-libbacktrace branch October 16, 2026 18:23
@stelfrag stelfrag mentioned this pull request Oct 26, 2026
Patch release 2.12.3 #24442
Merged
Ferroin pushed a commit that referenced this pull request Nov 4, 2026
@stelfrag @Ferroin
daemon: save the crash status file on builds without libbacktrace (#2…
8d1942d
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cubic-dev-ai cubic-dev-ai[bot] cubic-dev-ai[bot] left review comments
@thiagoftsm thiagoftsm thiagoftsm approved these changes
@vkalintiris vkalintiris Awaiting requested review from vkalintiris vkalintiris is a code owner
@Ferroin 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

Labels

area/build Build system (autotools and cmake). area/daemon area/packaging Packaging and operating systems support

Projects

None yet

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.

2 participants

@stelfrag @thiagoftsm
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.