@cryptotaxi247 / netdata-1 / commits / b194a287f

Fix memory-safety and correctness bugs surfaced by Coverity audit (part 7) (#22280)

* ml: lock host stats reset in detection pass Coverity CID 410121 (MISSING_LOCK): ml_host_detect_once() reset host->mls before taking host->mutex, while ml_host_stop() and the public readers access the same struct under that mutex. Move the reset into the existing critical section so host stats updates follow one lock discipline. * ml: narrow host mutex scope in detection stats Coverity CID 393056 (SLEEP): stop holding host->mutex across chart dictionary traversal in ml_host_detect_once(). Aggregate the host stats locally and publish them under the mutex so long chart-deletion waits on the rrdset index do not block ML readers and stop paths for the full walk. * ml: drop host mutex before stop chart walk Coverity CID 393057 (SLEEP): ml_host_stop() held host->mutex while waiting for the host chart dictionary read lock. Reset the host stats under the mutex and release it before walking the chart and dimension dictionaries so lengthy chart-deletion writers do not block ML readers behind the mutex. * ml: reject undersized kmeans training output Coverity CID 451560 (UNINIT): stop publishing a model when preprocessing yields fewer than two training vectors, because k-means returns with no cluster centers in that case. Also value-initialize inlined cluster centers so empty-source conversions stay deterministic instead of carrying indeterminate dlib matrix bytes. * Fix memory-safety issues in ml_kmeans_inlined_t initialization * Address review comments * Address review comments (2) * Address review comments (3) * Address review comments (4) * Address review comments (5) * ml: validate max_training_vectors and add stop generation counter Clamp max_training_vectors to [2, 86400] so a misconfigured value cannot force the undersampled early-return every cycle. Add ml_stop_generation on ml_host_t, bumped by ml_host_stop and sampled by ml_host_detect_once before and after the unlocked chart walk. A stop+start that completes within the walk window now invalidates the snapshot via the generation change even though ml_running is back to true at the re-check. * Adjust comment * ml: match master's post-cycle state in finalize helper Make ts = TRAINED unconditional in ml_dimension_finalize_constant_state to match ml_dimension_update_models. The earlier conditional was a no-op for stats anyway — ml_chart_update_dimension short-circuits on mt = CONSTANT before reading ts. Simplify the test accordingly. * ml: tighten stop generation ordering and review fixups Move ml_stop_generation.fetch_add to the end of ml_host_stop, after all chart->mls and dim resets, so a concurrent ml_host_detect_once that observes the new generation is guaranteed to also see the resets via seq_cst. The earlier ordering let detect sample the post-bump value mid-stop and miss the race. Also: take dim->slock in test_dimension_finalize_constant_state to match the helper's contract, and document the 86400 ceiling on max_training_vectors. * Fix compilation after rebase * ml: move ml_stop_generation initialization to ml_host_create * ml: simplify context_anomaly_rate iteration by removing unnecessary structured bindings * ml: add start_stop_mutex to synchronize ml_host_start and ml_host_stop operations Introduce `start_stop_mutex` to serialize `ml_host_start()` and `ml_host_stop()` operations, ensuring proper state handling during concurrent starts/stops. Update relevant mutex initialization and destruction, and adjust critical sections to prevent race conditions and ensure data consistency. --------- Co-authored-by: Costa Tsaousis <costa@netdata.cloud>

Stelios Fragkakis committed Jun 4, 2026 at 08:59 UTC b194a287f18dad41672442f3a43afbd33f24b27c

7 files changed +259 -46

src/ml/ml-unittest.cc

+108

@@ -1,6 +1,7 @@

1 // SPDX-License-Identifier: GPL-3.0-or-later

3 #include "ml_config.h"

4 +#include "ml_dimension.h"

5 #include "ml_features.h"

6 #include "ml_kmeans.h"

7 #include "ml_private.h"

@@ -365,6 +366,110 @@ static void test_kmeans_scoring()

366 ML_TEST_ASSERT_DOUBLE_EQ(score_eq, 0.0, 1e-9, "equal min/max should return 0");

367 }

368

369 +// Test: converting an empty ml_kmeans_t must produce deterministic zeroed centers.

370 +static void test_kmeans_inlined_empty_source_is_zero_initialized()

371 +{

372 + fprintf(stderr, " test_kmeans_inlined_empty_source_is_zero_initialized...\n");

373 +

374 + ml_kmeans_t empty_km;

375 + empty_km.cluster_centers.clear();

376 + empty_km.min_dist = 1.5;

377 + empty_km.max_dist = 9.5;

378 + empty_km.after = 11;

379 + empty_km.before = 22;

380 +

381 + ml_kmeans_inlined_t constructed(empty_km);

382 + ML_TEST_ASSERT(constructed.after == empty_km.after, "constructed empty model should preserve 'after'");

383 + ML_TEST_ASSERT(constructed.before == empty_km.before, "constructed empty model should preserve 'before'");

384 + ML_TEST_ASSERT_DOUBLE_EQ(constructed.min_dist, empty_km.min_dist, 0.0, "constructed empty model should preserve min_dist");

385 + ML_TEST_ASSERT_DOUBLE_EQ(constructed.max_dist, empty_km.max_dist, 0.0, "constructed empty model should preserve max_dist");

386 + for (size_t center = 0; center < constructed.cluster_centers.size(); center++) {

387 + ML_TEST_ASSERT(constructed.cluster_centers[center].size() == 6, "constructed empty-source centers must keep fixed-size geometry");

388 + for (long i = 0; i < constructed.cluster_centers[center].size(); i++) {

389 + char msg[160];

390 + snprintf(msg, sizeof(msg), "constructed empty-source center[%zu](%ld) should be zero", center, i);

391 + ML_TEST_ASSERT_DOUBLE_EQ(constructed.cluster_centers[center](i), 0.0, 0.0, msg);

392 + }

393 + }

394 +

395 + ml_kmeans_inlined_t assigned;

396 + for (int i = 0; i < 6; i++) {

397 + assigned.cluster_centers[0](i) = 10.0 + i;

398 + assigned.cluster_centers[1](i) = 20.0 + i;

399 + }

400 + assigned = empty_km;

401 +

402 + ML_TEST_ASSERT(assigned.after == empty_km.after, "assigned empty model should preserve 'after'");

403 + ML_TEST_ASSERT(assigned.before == empty_km.before, "assigned empty model should preserve 'before'");

404 + ML_TEST_ASSERT_DOUBLE_EQ(assigned.min_dist, empty_km.min_dist, 0.0, "assigned empty model should preserve min_dist");

405 + ML_TEST_ASSERT_DOUBLE_EQ(assigned.max_dist, empty_km.max_dist, 0.0, "assigned empty model should preserve max_dist");

406 + for (size_t center = 0; center < assigned.cluster_centers.size(); center++) {

407 + ML_TEST_ASSERT(assigned.cluster_centers[center].size() == 6, "empty-source centers must keep fixed-size geometry");

408 + for (long i = 0; i < assigned.cluster_centers[center].size(); i++) {

409 + char msg[160];

410 + snprintf(msg, sizeof(msg), "empty-source center[%zu](%ld) should be zero", center, i);

411 + ML_TEST_ASSERT_DOUBLE_EQ(assigned.cluster_centers[center](i), 0.0, 0.0, msg);

412 + }

413 + }

414 +}

415 +

416 +// Test: at the smallest legal input (src_n == diff_n + smooth_n + lag_n),

417 +// ml_features_preprocess yields exactly one feature vector. This guards the

418 +// preprocess boundary that triggers the <2-vectors early-return in

419 +// ml_dimension_train_model. Scope is intentionally limited to preprocess output:

420 +// ml_dimension_train_model itself depends on a live ml_dimension_t (worker, rd,

421 +// rrdset, host, sqlite) and is not unit-testable without significant plumbing.

422 +static void test_features_preprocess_below_min_for_kmeans()

423 +{

424 + fprintf(stderr, " test_features_preprocess_below_min_for_kmeans...\n");

425 +

426 + const size_t diff_n = 1;

427 + const size_t smooth_n = 1;

428 + const size_t lag_n = 1;

429 + const size_t src_n = diff_n + smooth_n + lag_n; // 3, the minimum allowed by ml_validate_features_input

430 +

431 + calculated_number_t src[16] = {1.0, 2.0, 3.0};

432 + calculated_number_t dst[16] = {0};

433 +

434 + std::vector<DSample> pf;

435 + ml_features_t features = {

436 + diff_n, smooth_n, lag_n,

437 + dst, src_n, src, src_n

438 + };

439 +

440 + ml_features_preprocess(&features, pf, 1.0);

441 +

442 + // n_vectors = src_n - diff_n - smooth_n + 1 - lag_n = 3 - 1 - 1 + 1 - 1 = 1

443 + ML_TEST_ASSERT(pf.size() == 1, "boundary input should yield exactly 1 feature vector");

444 + ML_TEST_ASSERT(pf.size() < 2, "<2 vectors must trigger the kmeans-skip early-return in ml_dimension_train_model");

445 +}

446 +

447 +// Test: ml_dimension_finalize_constant_state is the shared post-cycle state

448 +// transition used by both the successful-training path and the undersampled

449 +// early-return. It sets mt = CONSTANT, ts = TRAINED, and resets suppression

450 +// counters. Matches the existing master behavior in ml_dimension_update_models.

451 +static void test_dimension_finalize_constant_state()

452 +{

453 + fprintf(stderr, " test_dimension_finalize_constant_state...\n");

454 +

455 + ml_dimension_t dim = {};

456 + spinlock_init(&dim.slock);

457 + dim.mt = METRIC_TYPE_VARIABLE;

458 + dim.ts = TRAINING_STATUS_UNTRAINED;

459 + dim.suppression_anomaly_counter = 7;

460 + dim.suppression_window_counter = 13;

461 +

462 + // Match the helper's documented contract (caller holds dim->slock).

463 + spinlock_lock(&dim.slock);

464 + ml_dimension_finalize_constant_state(&dim);

465 + spinlock_unlock(&dim.slock);

466 +

467 + ML_TEST_ASSERT(dim.mt == METRIC_TYPE_CONSTANT, "mt must become CONSTANT");

468 + ML_TEST_ASSERT(dim.ts == TRAINING_STATUS_TRAINED, "ts must become TRAINED");

469 + ML_TEST_ASSERT(dim.suppression_anomaly_counter == 0, "anomaly counter must reset");

470 + ML_TEST_ASSERT(dim.suppression_window_counter == 0, "window counter must reset");

471 +}

472 +

473 // Test: circular buffer linearization produces the same result as std::rotate

474 static void test_circular_buffer_equivalence()

475 {

@@ -960,6 +1065,9 @@ extern "C" int ml_unittest()

1065 test_features_zero_smooth_matches_one();

1066 test_kmeans_scoring();

1067 test_full_pipeline();

1068 + test_kmeans_inlined_empty_source_is_zero_initialized();

1069 + test_features_preprocess_below_min_for_kmeans();

1070 + test_dimension_finalize_constant_state();

1071 test_circular_buffer_equivalence();

1072 test_same_value_uses_newest_sample();

1073 test_preprocess_predict_equivalence();

src/ml/ml.cc

+69 -26

@@ -726,6 +726,14 @@ bool ml_should_publish_model_update(bool host_running,

726 return true;

727 }

728

729 +void ml_dimension_finalize_constant_state(ml_dimension_t *dim)

730 +{

731 + dim->mt = METRIC_TYPE_CONSTANT;

732 + dim->ts = TRAINING_STATUS_TRAINED;

733 + dim->suppression_anomaly_counter = 0;

734 + dim->suppression_window_counter = 0;

735 +}

736 +

737 static bool ml_dimension_update_models(ml_worker_t *worker, ml_dimension_t *dim, uint32_t expected_generation, bool from_downstream)

738 {

739 worker_is_busy(WORKER_TRAIN_UPDATE_MODELS);

@@ -769,11 +777,7 @@ static bool ml_dimension_update_models(ml_worker_t *worker, ml_dimension_t *dim,

777 }

778 }

779

772 - dim->mt = METRIC_TYPE_CONSTANT;

773 - dim->ts = TRAINING_STATUS_TRAINED;

774 -

775 - dim->suppression_anomaly_counter = 0;

776 - dim->suppression_window_counter = 0;

780 + ml_dimension_finalize_constant_state(dim);

781

782 // Add the latest model to the list of pending models to flush.

783 ml_model_info_t model_info;

@@ -861,6 +865,17 @@ ml_dimension_train_model(ml_worker_t *worker, ml_dimension_t *dim)

865 // Apply sampling during lag feature extraction

866 ml_features_preprocess(&features, worker->training_samples, sampling_ratio);

867

868 + // Preprocessing can leave fewer than 2 vectors after diff/smooth/lag/sampling.

869 + // k-means cannot build 2 cluster centers from that input, so reuse the

870 + // post-cycle state machine and bail out before kmeans_train.

871 + if (worker->training_samples.size() < 2) {

872 + spinlock_lock(&dim->slock);

873 + ml_dimension_finalize_constant_state(dim);

874 + dim->training_in_progress = false;

875 + spinlock_unlock(&dim->slock);

876 + return ML_WORKER_RESULT_NOT_ENOUGH_COLLECTED_VALUES;

877 + }

878 +

879 ml_kmeans_init(&dim->kmeans);

880 ml_kmeans_train(&dim->kmeans, worker->training_samples, Cfg.max_kmeans_iters, training_response.query_after_t, training_response.query_before_t);

881 }

@@ -1076,11 +1091,15 @@ ml_host_detect_once(ml_host_t *host, ONEWAYALLOC *owa)

1091 {

1092 worker_is_busy(WORKER_JOB_DETECTION_COLLECT_STATS);

1093

1079 - host->mls = {};

1094 ml_machine_learning_stats_t mls_copy = {};

1095 + ml_machine_learning_stats_t host_mls = {};

1096 + calculated_number_t host_anomaly_rate = 0.0;

1097

1098 if (host->ml_running) {

1083 - netdata_mutex_lock(&host->mutex);

1099 + // Snapshot the stop generation before the unlocked walk. If it changes

1100 + // by the time we publish, a stop ran while we were reading chart->mls

1101 + // and the accumulated snapshot must be discarded.

1102 + uint64_t stop_gen_before = host->ml_stop_generation.load();

1103

1104 /*

1105 * prediction/detection stats

@@ -1098,20 +1117,20 @@ ml_host_detect_once(ml_host_t *host, ONEWAYALLOC *owa)

1117

1118 ml_machine_learning_stats_t chart_mls = chart->mls;

1119

1101 - host->mls.num_machine_learning_status_enabled += chart_mls.num_machine_learning_status_enabled;

1102 - host->mls.num_machine_learning_status_disabled_sp += chart_mls.num_machine_learning_status_disabled_sp;

1120 + host_mls.num_machine_learning_status_enabled += chart_mls.num_machine_learning_status_enabled;

1121 + host_mls.num_machine_learning_status_disabled_sp += chart_mls.num_machine_learning_status_disabled_sp;

1122

1104 - host->mls.num_metric_type_constant += chart_mls.num_metric_type_constant;

1105 - host->mls.num_metric_type_variable += chart_mls.num_metric_type_variable;

1123 + host_mls.num_metric_type_constant += chart_mls.num_metric_type_constant;

1124 + host_mls.num_metric_type_variable += chart_mls.num_metric_type_variable;

1125

1107 - host->mls.num_training_status_untrained += chart_mls.num_training_status_untrained;

1108 - host->mls.num_training_status_pending_without_model += chart_mls.num_training_status_pending_without_model;

1109 - host->mls.num_training_status_trained += chart_mls.num_training_status_trained;

1110 - host->mls.num_training_status_pending_with_model += chart_mls.num_training_status_pending_with_model;

1111 - host->mls.num_training_status_silenced += chart_mls.num_training_status_silenced;

1126 + host_mls.num_training_status_untrained += chart_mls.num_training_status_untrained;

1127 + host_mls.num_training_status_pending_without_model += chart_mls.num_training_status_pending_without_model;

1128 + host_mls.num_training_status_trained += chart_mls.num_training_status_trained;

1129 + host_mls.num_training_status_pending_with_model += chart_mls.num_training_status_pending_with_model;

1130 + host_mls.num_training_status_silenced += chart_mls.num_training_status_silenced;

1131

1113 - host->mls.num_anomalous_dimensions += chart_mls.num_anomalous_dimensions;

1114 - host->mls.num_normal_dimensions += chart_mls.num_normal_dimensions;

1132 + host_mls.num_anomalous_dimensions += chart_mls.num_anomalous_dimensions;

1133 + host_mls.num_normal_dimensions += chart_mls.num_normal_dimensions;

1134

1135 if (spinlock_trylock(&host->context_anomaly_rate_spinlock))

1136 {

@@ -1137,20 +1156,44 @@ ml_host_detect_once(ml_host_t *host, ONEWAYALLOC *owa)

1156 }

1157 rrdset_foreach_done(rsp);

1158

1140 - host->host_anomaly_rate = 0.0;

1141 - size_t NumActiveDimensions = host->mls.num_anomalous_dimensions + host->mls.num_normal_dimensions;

1142 - if (NumActiveDimensions)

1143 - host->host_anomaly_rate = static_cast<double>(host->mls.num_anomalous_dimensions) / NumActiveDimensions;

1144 -

1145 - mls_copy = host->mls;

1146 -

1159 + size_t num_active_dimensions = host_mls.num_anomalous_dimensions + host_mls.num_normal_dimensions;

1160 + if (num_active_dimensions)

1161 + host_anomaly_rate = static_cast<double>(host_mls.num_anomalous_dimensions) / num_active_dimensions;

1162 +

1163 + // Publish the final host snapshot after chart traversal so chart

1164 + // deletion cannot block other host->mutex users for the full walk.

1165 + // Discard the snapshot if either (a) ml_running is now false, or

1166 + // (b) the stop generation changed since the walk started — the

1167 + // latter catches a stop+start that completed during the walk and

1168 + // would otherwise pass the boolean check. In either case our

1169 + // unlocked chart->mls reads may have raced ml_host_stop, so zero

1170 + // the snapshot and the per-context counts. The chart updates below

1171 + // run unconditionally: ml_update_dimensions_chart reads

1172 + // host->ml_running directly (so the ml_running chart records the

1173 + // stop), and the chart-update path resets and republishes the rest.

1174 + netdata_mutex_lock(&host->mutex);

1175 + uint64_t stop_gen_after = host->ml_stop_generation.load();

1176 + if (!host->ml_running || stop_gen_before != stop_gen_after) {

1177 + host_mls = {};

1178 + host_anomaly_rate = 0.0;

1179 +

1180 + spinlock_lock(&host->context_anomaly_rate_spinlock);

1181 + for (auto &p : host->context_anomaly_rate) {

1182 + p.second.anomalous_dimensions = 0;

1183 + p.second.normal_dimensions = 0;

1184 + }

1185 + spinlock_unlock(&host->context_anomaly_rate_spinlock);

1186 + }

1187 + host->mls = host_mls;

1188 + host->host_anomaly_rate = host_anomaly_rate;

1189 + mls_copy = host_mls;

1190 netdata_mutex_unlock(&host->mutex);

1191

1192 worker_is_busy(WORKER_JOB_DETECTION_DIM_CHART);

1193 ml_update_dimensions_chart(host, mls_copy);

1194

1195 worker_is_busy(WORKER_JOB_DETECTION_HOST_CHART);

1153 - ml_update_host_and_detection_rate_charts(host, host->host_anomaly_rate * 10000.0, owa);

1196 + ml_update_host_and_detection_rate_charts(host, host_anomaly_rate * 10000.0, owa);

1197 } else {

1198 host->host_anomaly_rate = 0.0;

1199 }

src/ml/ml_config.cc

@@ -173,6 +173,13 @@ void ml_config_load(ml_config_t *cfg) {

173 diff_n = clamp(diff_n, 0u, 1u);

174 max_samples_to_smooth = clamp<size_t>(max_samples_to_smooth, 0, 5);

175 lag_n = clamp(lag_n, 1u, 5u);

176 + // max_training_vectors drives the lag-extraction sampling ratio (not a

177 + // hard cap on output). A floor of 2 keeps the sampler from being starved

178 + // by a misconfigured value; the ceiling of 86400 (24 hours of 1-second

179 + // samples) is well past anything training_window can supply. The runtime

180 + // guard in ml_dimension_train_model is still required because sampling is

181 + // probabilistic and can drop the emitted vector count below 2.

182 + max_training_vectors = clamp<size_t>(max_training_vectors, 2, 86400);

183

184 max_kmeans_iters = clamp(max_kmeans_iters, 500u, 1000u);

185

src/ml/ml_dimension.h

@@ -35,6 +35,11 @@ ml_dimension_predict(ml_dimension_t *dim, calculated_number_t value, bool exists

36 bool ml_dimension_deserialize_kmeans(const char *json_str);

38 +// Set dim's post-training state (mt/ts/suppression counters). Caller must hold

39 +// dim->slock. Used by both the successful-training path and the undersampled

40 +// early-return so the post-cycle state machine stays in sync.

41 +void ml_dimension_finalize_constant_state(ml_dimension_t *dim);

42 +

43 class DimensionLookupInfo {

44 public:

45 DimensionLookupInfo()

src/ml/ml_host.h

+16

@@ -40,12 +40,28 @@ typedef struct {

41 std::atomic<bool> ml_running;

43 + // Incremented at the END of every ml_host_stop, after all chart/dim resets

44 + // have committed. ml_host_detect_once samples this before and after its

45 + // unlocked chart walk; a change means a stop completed during the walk

46 + // (so detect raced with stop's chart->mls writes) or a stop+start cycle

47 + // happened around the walk. In either case the accumulated snapshot must

48 + // be discarded even if ml_running is back to true at the re-check.

49 + std::atomic<uint64_t> ml_stop_generation;

50 +

51 ml_machine_learning_stats_t mls;

53 calculated_number_t host_anomaly_rate;

55 netdata_mutex_t mutex;

57 + // Serializes ml_host_start() against ml_host_stop(). Stop holds it across

58 + // its full chart/dim reset walk and the final stop-generation bump (it

59 + // cannot carry host->mutex into that walk), so a racing start cannot

60 + // re-enable ml_running while stop is mid-reset. Without it, a detect walk

61 + // could observe ml_running==true with an unchanged stop generation and

62 + // publish a snapshot torn by stop's in-flight chart->mls resets.

63 + netdata_mutex_t start_stop_mutex;

64 +

65 ml_queue_t *queue;

67 /*

src/ml/ml_kmeans.h

+11 -6

@@ -35,19 +35,20 @@ struct ml_kmeans_inlined_t {

36 ml_kmeans_inlined_t() : min_dist(0), max_dist(0), after(0), before(0)

37 {

38 + cluster_centers[0] = 0;

39 + cluster_centers[1] = 0;

40 }

40 - explicit ml_kmeans_inlined_t(const ml_kmeans_t &km)

42 + explicit ml_kmeans_inlined_t(const ml_kmeans_t &km) : min_dist(km.min_dist), max_dist(km.max_dist), after(km.after), before(km.before)

43 {

44 if (km.cluster_centers.size() == 2) {

45 cluster_centers[0] = km.cluster_centers[0];

46 cluster_centers[1] = km.cluster_centers[1];

47 }

46 -

47 - min_dist = km.min_dist;

48 - max_dist = km.max_dist;

49 - after = km.after;

50 - before = km.before;

48 + else {

49 + cluster_centers[0] = 0;

50 + cluster_centers[1] = 0;

51 + }

52 }

54 ml_kmeans_inlined_t &operator=(const ml_kmeans_t &km)

@@ -56,6 +57,10 @@ struct ml_kmeans_inlined_t {

57 cluster_centers[0] = km.cluster_centers[0];

58 cluster_centers[1] = km.cluster_centers[1];

59 }

60 + else {

61 + cluster_centers[0] = 0;

62 + cluster_centers[1] = 0;

63 + }

64 min_dist = km.min_dist;

65 max_dist = km.max_dist;

66 after = km.after;

src/ml/ml_public.cc

+43 -14

@@ -92,9 +92,11 @@ void ml_host_new(RRDHOST *rh)

92 host->queue = Cfg.workers[times_called++ % Cfg.num_worker_threads].queue;

94 netdata_mutex_init(&host->mutex);

95 + netdata_mutex_init(&host->start_stop_mutex);

96 spinlock_init(&host->context_anomaly_rate_spinlock);

98 host->ml_running = false;

99 + host->ml_stop_generation = 0;

100

101 // Publish with release semantics so readers that load rh->ml_host with

102 // acquire semantics observe the host's `rh`, `ml_running`, `mutex`,

@@ -120,6 +122,7 @@ void ml_host_delete(RRDHOST *rh)

122

123 ml_host_clear_context_anomaly_rate(host);

124 netdata_mutex_destroy(&host->mutex);

125 + netdata_mutex_destroy(&host->start_stop_mutex);

126

127 delete host;

128 }

@@ -129,17 +132,20 @@ void ml_host_start(RRDHOST *rh) {

132 if (!host)

133 return;

134

132 - // Set ml_running and run the sweep under host->mutex so concurrent

133 - // ml_host_start() calls are serialized and the visibility window of the

134 - // flag flip is bounded by the same critical section that performs the

135 - // sweep.

136 - netdata_mutex_lock(&host->mutex);

135 + // Serialize against ml_host_stop(): we must not re-enable ml_running

136 + // while a stop is still resetting chart/dim state (see ml_host_stop),

137 + // and concurrent ml_host_start() calls must not run the sweep twice.

138 + netdata_mutex_lock(&host->start_stop_mutex);

139

140 if (host->ml_running) {

139 - netdata_mutex_unlock(&host->mutex);

141 + netdata_mutex_unlock(&host->start_stop_mutex);

142 return;

143 }

144

145 + // Run the sweep under host->mutex so the visibility window of the flag

146 + // flip is bounded by the same critical section that performs the sweep.

147 + netdata_mutex_lock(&host->mutex);

148 +

149 host->ml_running = true;

150

151 void *rsp = NULL;

@@ -156,27 +162,43 @@ void ml_host_start(RRDHOST *rh) {

162 rrdset_foreach_done(rsp);

163

164 netdata_mutex_unlock(&host->mutex);

165 + netdata_mutex_unlock(&host->start_stop_mutex);

166 }

167

168 void ml_host_stop(RRDHOST *rh) {

169 ml_host_t *host = (ml_host_t *) __atomic_load_n(&rh->ml_host, __ATOMIC_ACQUIRE);

163 - if (!host || !host->ml_running)

170 + if (!host)

171 return;

172

166 - // Prevent new ML activity from publishing while we reset host/dimension state.

167 - host->ml_running = false;

173 + // Serialize with ml_host_start() for the WHOLE stop sequence, including

174 + // the unlocked chart/dim reset walk and the final generation bump. If a

175 + // racing start could flip ml_running back to true mid-reset, a concurrent

176 + // ml_host_detect_once would observe ml_running==true with an unchanged

177 + // stop generation and publish a snapshot torn by our in-flight resets.

178 + netdata_mutex_lock(&host->start_stop_mutex);

179

169 - netdata_mutex_lock(&host->mutex);

180 + if (!host->ml_running) {

181 + netdata_mutex_unlock(&host->start_stop_mutex);

182 + return;

183 + }

184

171 - // Re-assert under the mutex so stop deterministically wins over a racing

172 - // ml_host_start() that may have flipped the flag back to true after our

173 - // early write but before we acquired the mutex.

185 + // Prevent new ML activity from publishing while we reset host/dimension

186 + // state. The ml_running flag gates collectors and the detect loop; the

187 + // stop generation is bumped at the end of the function so a concurrent

188 + // ml_host_detect_once that observes the new generation is guaranteed to

189 + // also see all of our chart->mls / dim resets via seq_cst ordering.

190 host->ml_running = false;

191

192 + netdata_mutex_lock(&host->mutex);

193 +

194 // reset host stats

195 host->mls = ml_machine_learning_stats_t();

196 ml_host_clear_context_anomaly_rate(host);

197

198 + // Chart deletion can hold the dictionary writer across lengthy cleanup.

199 + // Do not carry host->mutex into the traversal below.

200 + netdata_mutex_unlock(&host->mutex);

201 +

202 // reset charts/dims

203 void *rsp = NULL;

204 rrdset_foreach_read(rsp, host->rh) {

@@ -218,7 +240,14 @@ void ml_host_stop(RRDHOST *rh) {

240 }

241 rrdset_foreach_done(rsp);

242

221 - netdata_mutex_unlock(&host->mutex);

243 + // Publish the stop only after every chart->mls / dim reset is committed.

244 + // ml_host_detect_once treats a generation change as "discard the snapshot",

245 + // so bumping here guarantees that if detect saw stale chart->mls it will

246 + // either also observe the new generation or have already published before

247 + // any of our resets started.

248 + host->ml_stop_generation.fetch_add(1);

249 +

250 + netdata_mutex_unlock(&host->start_stop_mutex);

251 }

252

253 void ml_host_get_info(RRDHOST *rh, BUFFER *wb)