@cryptotaxi247 / netdata-1 / commits / 336738b33

Fix SIGSEGV (contexts) (#22025)

thiagoftsm committed Mar 26, 2026 at 13:38 UTC 336738b33b4c0f5ff0f8d0efdb5cabb970334823
6 files changed +79 -22
src/database/contexts/query_target.c
+10 -3
@@ -269,13 +269,15 @@ static bool query_metric_add(QUERY_TARGET_LOCALS *qtl, QUERY_NODE *qn, QUERY_CON
269 time_t db_update_every_s;
270 } tier_retention[nd_profile.storage_tiers];
271
272 + RRDDIM *rd = rrdmetric_rrddim_get_and_lock(rm);
273 +
274 for (size_t tier = 0; tier < nd_profile.storage_tiers; tier++) {
275 STORAGE_ENGINE *eng = qn->rrdhost->db[tier].eng;
276 tier_retention[tier].eng = eng;
277 tier_retention[tier].db_update_every_s = (time_t) (qn->rrdhost->db[tier].tier_grouping * ri->update_every_s);
278
277 - if(rm->rrddim && rm->rrddim->tiers[tier].smh)
278 - tier_retention[tier].smh = eng->api.metric_dup(rm->rrddim->tiers[tier].smh);
279 + if(rd && rd->tiers[tier].smh)
280 + tier_retention[tier].smh = eng->api.metric_dup(rd->tiers[tier].smh);
281 else
282 tier_retention[tier].smh = eng->api.metric_get_by_id(qn->rrdhost->db[tier].si, rm->uuid);
283
@@ -307,6 +309,8 @@ static bool query_metric_add(QUERY_TARGET_LOCALS *qtl, QUERY_NODE *qn, QUERY_CON
309 }
310 }
311
312 + rrdmetric_rrddim_unlock(rd);
313 +
314 for (size_t tier = 0; tier < nd_profile.storage_tiers; tier++) {
315 if(!qt->db.tiers[tier].update_every || (tier_retention[tier].db_update_every_s && tier_retention[tier].db_update_every_s < qt->db.tiers[tier].update_every))
316 qt->db.tiers[tier].update_every = tier_retention[tier].db_update_every_s;
@@ -481,8 +485,11 @@ static bool query_dimension_add(QUERY_TARGET_LOCALS *qtl, QUERY_NODE *qn, QUERY_
485 // we don't have a dimensions pattern
486 // so this is a selected dimension
487 // if it is not hidden
488 + RRDDIM *rd = rrdmetric_rrddim_get_and_lock(rm);
489 + bool hidden = rrd_flag_check(rm, RRD_FLAG_HIDDEN) || (rd && rrddim_option_check(rd, RRDDIM_OPTION_HIDDEN));
490 + rrdmetric_rrddim_unlock(rd);
491
485 - if(rrd_flag_check(rm, RRD_FLAG_HIDDEN) || (rm->rrddim && rrddim_option_check(rm->rrddim, RRDDIM_OPTION_HIDDEN))) {
492 + if(hidden) {
493 // this is a hidden dimension
494 // we don't need to query it
495 status |= QUERY_STATUS_DIMENSION_HIDDEN;
src/database/contexts/rrdcontext-instance.c
+1 -1
@@ -349,7 +349,7 @@ inline void rrdinstance_from_rrdset(RRDSET *st) {
349
350 RRDMETRIC *rm_old = rrdmetric_acquired_value(rd->rrdcontexts.rrdmetric);
351 rrdmetric_set_deleted_overwrite(rm_old, RRD_FLAG_UPDATED|RRD_FLAG_LIVE_RETENTION|RRD_FLAG_UPDATE_REASON_UNUSED|RRD_FLAG_UPDATE_REASON_ZERO_RETENTION);
352 - rm_old->rrddim = NULL;
352 + rrdmetric_rrddim_atomic_store(rm_old, NULL);
353 rm_old->first_time_s = 0;
354 rm_old->last_time_s = 0;
355
src/database/contexts/rrdcontext-internal.h
+39 -2
@@ -227,6 +227,42 @@ typedef struct rrdmetric {
227 struct rrdinstance *ri;
228 } RRDMETRIC;
229
230 +static ALWAYS_INLINE RRDDIM *rrdmetric_rrddim_atomic_load(RRDMETRIC *rm) {
231 + return __atomic_load_n(&rm->rrddim, __ATOMIC_ACQUIRE);
232 +}
233 +
234 +static ALWAYS_INLINE void rrdmetric_rrddim_atomic_store(RRDMETRIC *rm, RRDDIM *rd) {
235 + __atomic_store_n(&rm->rrddim, rd, __ATOMIC_RELEASE);
236 +}
237 +
238 +static ALWAYS_INLINE RRDDIM *rrdmetric_rrddim_get_and_lock(RRDMETRIC *rm) {
239 + for(size_t retries = 0; retries < 5; retries++) {
240 + RRDDIM *rd = rrdmetric_rrddim_atomic_load(rm);
241 + if(unlikely(!rd))
242 + return NULL;
243 +
244 + if(unlikely(!spinlock_trylock(&rd->destroy_lock))) {
245 + if(retries + 1 < 5)
246 + microsleep(1 * USEC_PER_MS);
247 + continue;
248 + }
249 +
250 + if(unlikely(rrdmetric_rrddim_atomic_load(rm) != rd)) {
251 + spinlock_unlock(&rd->destroy_lock);
252 + continue;
253 + }
254 +
255 + return rd;
256 + }
257 +
258 + return NULL;
259 +}
260 +
261 +static ALWAYS_INLINE void rrdmetric_rrddim_unlock(RRDDIM *rd) {
262 + if(likely(rd))
263 + spinlock_unlock(&rd->destroy_lock);
264 +}
265 +
266 typedef struct rrdinstance {
267 UUIDMAP_ID uuid;
268 int update_every_s; // data collection frequency
@@ -311,8 +347,9 @@ static ALWAYS_INLINE void rrdmetric_set_collected(RRDMETRIC *rm) {
347 if(!(old & RRD_FLAG_COLLECTED))
348 __atomic_add_fetch(&rm->ri->rc->rrdhost->collected.metrics_count, 1, __ATOMIC_RELAXED);
349
314 - if(likely(rm->rrddim))
315 - rm->rrddim->rrdcontexts.collected = true;
350 + RRDDIM *rd = rrdmetric_rrddim_atomic_load(rm);
351 + if(likely(rd))
352 + rd->rrdcontexts.collected = true;
353 }
354
355 static ALWAYS_INLINE void rrdmetric_set_archived(RRDMETRIC *rm) {
src/database/contexts/rrdcontext-metric.c
+17 -11
@@ -31,9 +31,12 @@ inline STRING *rrdmetric_acquired_name_dup(RRDMETRIC_ACQUIRED *rma) {
31
32 inline NETDATA_DOUBLE rrdmetric_acquired_last_stored_value(RRDMETRIC_ACQUIRED *rma) {
33 RRDMETRIC *rm = rrdmetric_acquired_value(rma);
34 -
35 - if(rm->rrddim)
36 - return rm->rrddim->collector.last_stored_value;
34 + RRDDIM *rd = rrdmetric_rrddim_get_and_lock(rm);
35 + if(rd) {
36 + NETDATA_DOUBLE last_stored_value = rd->collector.last_stored_value;
37 + rrdmetric_rrddim_unlock(rd);
38 + return last_stored_value;
39 + }
40
41 return NAN;
42 }
@@ -97,7 +100,7 @@ static void rrdmetric_insert_callback(const DICTIONARY_ITEM *item __maybe_unused
100 static void rrdmetric_delete_callback(const DICTIONARY_ITEM *item __maybe_unused, void *value, void *rrdinstance __maybe_unused) {
101 RRDMETRIC *rm = value;
102
100 - internal_error(rm->rrddim, "RRDMETRIC: '%s' is freed but there is a RRDDIM linked to it.", string2str(rm->id));
103 + internal_error(rrdmetric_rrddim_atomic_load(rm), "RRDMETRIC: '%s' is freed but there is a RRDDIM linked to it.", string2str(rm->id));
104
105 // update the count of metrics
106 __atomic_sub_fetch(&rm->ri->rc->rrdhost->rrdctx.metrics_count, 1, __ATOMIC_RELAXED);
@@ -151,13 +154,16 @@ static bool rrdmetric_conflict_callback(const DICTIONARY_ITEM *item __maybe_unus
154 rrd_flag_set_updated(rm, RRD_FLAG_UPDATE_REASON_CHANGED_METADATA);
155 }
156
154 - if(rm->rrddim && rm_new->rrddim && rm->rrddim != rm_new->rrddim) {
155 - rm->rrddim = rm_new->rrddim;
157 + RRDDIM *rd = rrdmetric_rrddim_atomic_load(rm);
158 + RRDDIM *rd_new = rrdmetric_rrddim_atomic_load(rm_new);
159 +
160 + if(rd && rd_new && rd != rd_new) {
161 + rrdmetric_rrddim_atomic_store(rm, rd_new);
162 rrd_flag_set_updated(rm, RRD_FLAG_UPDATE_REASON_CHANGED_LINKING);
163 }
164
159 - if(rm->rrddim != rm_new->rrddim)
160 - rm->rrddim = rm_new->rrddim;
165 + if(rd != rd_new)
166 + rrdmetric_rrddim_atomic_store(rm, rd_new);
167
168 if(rm->name != rm_new->name) {
169 SWAP(rm->name, rm_new->name);
@@ -216,7 +222,7 @@ void rrdmetrics_destroy_from_rrdinstance(RRDINSTANCE *ri) {
222
223 // trigger post-processing of the rrdmetric, escalating changes to the rrdinstance it belongs
224 void rrdmetric_trigger_updates(RRDMETRIC *rm, const char *function) {
219 - if(unlikely(rrd_flag_is_collected(rm)) && (!rm->rrddim || rrd_flag_check(rm, RRD_FLAG_UPDATE_REASON_DISCONNECTED_CHILD)))
225 + if(unlikely(rrd_flag_is_collected(rm)) && (!rrdmetric_rrddim_atomic_load(rm) || rrd_flag_check(rm, RRD_FLAG_UPDATE_REASON_DISCONNECTED_CHILD)))
226 rrdmetric_set_archived(rm);
227
228 if(rrd_flag_is_updated(rm) || !rrd_flag_check(rm, RRD_FLAG_LIVE_RETENTION)) {
@@ -274,7 +280,7 @@ static ALWAYS_INLINE RRDMETRIC *rrddim_get_rrdmetric_with_trace(RRDDIM *rd, cons
280 return NULL;
281 }
282
277 - if(unlikely(rm->rrddim != rd))
283 + if(unlikely(rrdmetric_rrddim_atomic_load(rm) != rd))
284 fatal("RRDMETRIC: '%s' is not linked to RRDDIM '%s' at %s()", string2str(rm->id), rrddim_id(rd), function);
285
286 return rm;
@@ -287,7 +293,7 @@ inline void rrdmetric_rrddim_is_freed(RRDDIM *rd) {
293 if(unlikely(rrd_flag_is_collected(rm)))
294 rrdmetric_set_archived(rm);
295
290 - rm->rrddim = NULL;
296 + rrdmetric_rrddim_atomic_store(rm, NULL);
297 rrdmetric_trigger_updates(rm, __FUNCTION__ );
298 rrdmetric_release(rd->rrdcontexts.rrdmetric);
299 rd->rrdcontexts.rrdmetric = NULL;
src/database/contexts/rrdcontext-worker.c
+7 -5
@@ -168,10 +168,12 @@ void get_metric_retention_by_id(RRDHOST *host, UUIDMAP_ID id, time_t *min_first_
168
169 bool rrdmetric_update_retention(RRDMETRIC *rm) {
170 time_t min_first_time_t = LONG_MAX, max_last_time_t = 0;
171 + RRDDIM *rd = rrdmetric_rrddim_get_and_lock(rm);
172
172 - if(rm->rrddim) {
173 - min_first_time_t = rrddim_first_entry_s(rm->rrddim);
174 - max_last_time_t = rrddim_last_entry_s(rm->rrddim);
173 + if(rd) {
174 + min_first_time_t = rrddim_first_entry_s(rd);
175 + max_last_time_t = rrddim_last_entry_s(rd);
176 + rrdmetric_rrddim_unlock(rd);
177 rrd_flag_clear(rm, RRD_FLAG_NO_TIER0_RETENTION);
178 }
179 else {
@@ -219,7 +221,7 @@ static inline bool rrdmetric_should_be_deleted(RRDMETRIC *rm) {
221 if(likely(rrd_flag_check(rm, RRD_FLAGS_PREVENTING_DELETIONS)))
222 return false;
223
222 - if(likely(rm->rrddim))
224 + if(likely(rrdmetric_rrddim_atomic_load(rm)))
225 return false;
226
227 rrdmetric_update_retention(rm);
@@ -540,7 +542,7 @@ static bool rrdinstance_forcefully_clear_retention(RRDCONTEXT *rc, size_t count,
542 size_t metrics_cleared = 0;
543 RRDMETRIC *rm;
544 dfe_start_read(ri->rrdmetrics, rm) {
543 - if(!rrd_flag_check(rm, RRD_FLAG_NO_TIER0_RETENTION) || rrd_flag_is_collected(rm) || rm->rrddim)
545 + if(!rrd_flag_check(rm, RRD_FLAG_NO_TIER0_RETENTION) || rrd_flag_is_collected(rm) || rrdmetric_rrddim_atomic_load(rm))
546 continue;
547
548 rrdmetric_update_retention(rm);
src/libnetdata/aral/aral.c
+5
@@ -998,6 +998,11 @@ void aral_freez_internal(ARAL *ar, void *ptr TRACE_ALLOCATIONS_FUNCTION_DEFINITI
998 bool marked;
999 ARAL_PAGE *page = aral_get_page_pointer_after_element___do_NOT_have_aral_lock(ar, ptr, &marked);
1000
1001 + // Clear the embedded trailer before publishing the slot back to the free lists.
1002 + // This makes stale/double frees fail through the null-page check instead of
1003 + // dereferencing a stale ARAL_PAGE pointer while the slot is idle.
1004 + aral_set_page_pointer_after_element___do_NOT_have_aral_lock(ar, NULL, ptr, false);
1005 +
1006 size_t idx = mark_to_idx(marked);
1007 __atomic_add_fetch(&ar->ops[idx].atomic.deallocators, 1, __ATOMIC_RELAXED);
1008