No issues found across 1 file

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

sequenceDiagram
    participant Svc as Service/Cleanup Thread
    participant Coll as Collector Thread
    participant RRD as RRDSET (Chart Metadata)
    participant Mem as PRD Dimension Memory

    Note over Svc,Mem: Cleanup Request Flow (rrdset_pluginsd_receive_unslot_and_cleanup)

    Svc->>RRD: atomic_load(collector_tid)
    RRD-->>Svc: returns active_tid

    alt NEW: collector_tid != current_tid (Race Prevention)
        Note right of Svc: Another thread owns the chart.
        Svc->>Svc: Log warning (Skipping cleanup)
        Note over Svc,RRD: CHANGED: We no longer rely on<br/>RRDSET_FLAG_COLLECTION_FINISHED<br/>as it was prone to races.
    else CHANGED: collector_tid == current_tid (Collector teardown)
        Svc->>Svc: internal_fatal() (if NETDATA_INTERNAL_CHECKS)
        Svc->>RRD: NEW: atomic_store(collector_tid, 0)
        Svc->>Mem: Free PRD dimension references
    else collector_tid == 0 (Safe State)
        Svc->>Mem: Free PRD dimension references
    end

    Note over Coll,Mem: Active Collection Flow (concurrent)
    Coll->>RRD: Access chart dimensions
    opt collector_tid != 0
        Coll->>Mem: Dereference pointers
        Note left of Mem: PRD references remain valid because<br/>Service Thread skipped cleanup.
    end

Pull request overview

Fixes a race in pluginsd slot cleanup where PRD dimension references could be released while an active collector thread may still be dereferencing cached pointers, by tightening collector thread ownership checks during cleanup.

Changes:

• Skip rrdset_pluginsd_receive_unslot_and_cleanup() when collector_tid is set and owned by a different thread (no longer relying on RRDSET_FLAG_COLLECTION_FINISHED as a gate).
• When cleanup is invoked from the collector thread, explicitly log/assert and force-clear collector_tid before proceeding.
• Remove the prior “stale collector_tid handoff” path and clarify inline comments to avoid lifecycle violations.

src/database/rrdset-slots.c:229

• This warning is emitted when cleanup is skipped due to an active collector, but it doesn't include chart/host identification, which can make it hard to action (especially if this can happen during shutdown/finalization across many charts). Consider including rrdhost_hostname(st->rrdhost) and rrdset_id(st) (or similar identifiers) in the log fields/message.

            nd_log_limit(&erl, NDLS_DAEMON, NDLP_WARNING,
                         "PLUGINSD: attempted cleanup while collector (tid %d) is still active on chart, skipping",
                         collector_tid);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

PR is running as expected during few hours. LGTM!