@cryptotaxi247 / netdata-1 / commits / 79444d364

fix memory leaks and mismatches of the use of the z functions for allocations (#12841)

* fix mismatches of the use of the z functions for allocations * when there was no memory; the original name of the dimensions was freed, and with mismatching deallocator.. * fixed memory leak at rrdeng_load_metric_*() functions * fixed memory leak on exit of plugins.d parser * fixed memory leak on plugins and streaming receiver threads exit * fixed compiler warnings

Costa Tsaousis committed May 7, 2022 at 23:00 UTC 79444d36459b105f093c5626eea8f0b45af7f421

10 files changed +93 -97

collectors/plugins.d/pluginsd_parser.c

+22 -19

@@ -725,6 +725,11 @@ PARSER_RC metalog_pluginsd_host(char **words, void *user, PLUGINSD_ACTION *plug

725 return PARSER_RC_OK;

726 }

727

728 +static void pluginsd_process_thread_cleanup(void *ptr) {

729 + PARSER *parser = (PARSER *)ptr;

730 + parser_destroy(parser);

731 +}

732 +

733 // New plugins.d parser

734

735 inline size_t pluginsd_process(RRDHOST *host, struct plugind *cd, FILE *fp, int trust_durations)

@@ -743,19 +748,18 @@ inline size_t pluginsd_process(RRDHOST *host, struct plugind *cd, FILE *fp, int

748 }

749 clearerr(fp);

750

746 - PARSER_USER_OBJECT *user = callocz(1, sizeof(*user));

747 - ((PARSER_USER_OBJECT *) user)->enabled = cd->enabled;

748 - ((PARSER_USER_OBJECT *) user)->host = host;

749 - ((PARSER_USER_OBJECT *) user)->cd = cd;

750 - ((PARSER_USER_OBJECT *) user)->trust_durations = trust_durations;

751 + PARSER_USER_OBJECT user = {

752 + .enabled = cd->enabled,

753 + .host = host,

754 + .cd = cd,

755 + .trust_durations = trust_durations

756 + };

757

752 - PARSER *parser = parser_init(host, user, fp, PARSER_INPUT_SPLIT);

758 + PARSER *parser = parser_init(host, &user, fp, PARSER_INPUT_SPLIT);

759

754 - if (unlikely(!parser)) {

755 - error("Failed to initialize parser");

756 - cd->serial_failures++;

757 - return 0;

758 - }

760 + // this keeps the parser with its current value

761 + // so, parser needs to be allocated before pushing it

762 + netdata_thread_cleanup_push(pluginsd_process_thread_cleanup, parser);

763

764 parser->plugins_action->begin_action = &pluginsd_begin_action;

765 parser->plugins_action->flush_action = &pluginsd_flush_action;

@@ -770,25 +774,24 @@ inline size_t pluginsd_process(RRDHOST *host, struct plugind *cd, FILE *fp, int

774 parser->plugins_action->clabel_commit_action = &pluginsd_clabel_commit_action;

775 parser->plugins_action->clabel_action = &pluginsd_clabel_action;

776

773 - user->parser = parser;

777 + user.parser = parser;

778

779 while (likely(!parser_next(parser))) {

780 if (unlikely(netdata_exit || parser_action(parser, NULL)))

781 break;

782 }

779 - info("PARSER ended");

780 -

781 - parser_destroy(parser);

783

783 - cd->enabled = ((PARSER_USER_OBJECT *) user)->enabled;

784 - size_t count = ((PARSER_USER_OBJECT *) user)->count;

784 + // free parser with the pop function

785 + netdata_thread_cleanup_pop(1);

786

786 - freez(user);

787 + cd->enabled = user.enabled;

788 + size_t count = user.count;

789

790 if (likely(count)) {

791 cd->successful_collections += count;

792 cd->serial_failures = 0;

791 - } else

793 + }

794 + else

795 cd->serial_failures++;

796

797 return count;

collectors/proc.plugin/proc_spl_kstat_zfs.c

+1 -1

@@ -290,7 +290,7 @@ int update_zfs_pool_state_chart(char *name, void *pool_p, void *update_every_p)

290 }

291 } else {

292 disable_zfs_pool_state(pool);

293 - struct deleted_zfs_pool *new = calloc(1, sizeof(struct deleted_zfs_pool));

293 + struct deleted_zfs_pool *new = callocz(1, sizeof(struct deleted_zfs_pool));

294 new->name = strdupz(name);

295 new->next = deleted_zfs_pools;

296 deleted_zfs_pools = new;

daemon/main.c

+1 -1

@@ -1235,7 +1235,7 @@ int main(int argc, char **argv) {

1235 // initialize rrd, registry, health, rrdpush, etc.

1236

1237 netdata_anonymous_statistics_enabled=-1;

1238 - struct rrdhost_system_info *system_info = calloc(1, sizeof(struct rrdhost_system_info));

1238 + struct rrdhost_system_info *system_info = callocz(1, sizeof(struct rrdhost_system_info));

1239 get_system_info(system_info);

1240 system_info->hops = 0;

1241 get_install_type(&system_info->install_type, &system_info->prebuilt_arch, &system_info->prebuilt_dist);

database/engine/metadata_log/logfile.c

+7 -13

@@ -375,19 +375,15 @@ static int scan_metalog_files(struct metalog_instance *ctx)

375 struct metalog_pluginsd_state metalog_parser_state;

376 metalog_pluginsd_state_init(&metalog_parser_state, ctx);

377

378 - PARSER_USER_OBJECT metalog_parser_object;

379 - metalog_parser_object.enabled = cd.enabled;

380 - metalog_parser_object.host = ctx->rrdeng_ctx->host;

381 - metalog_parser_object.cd = &cd;

382 - metalog_parser_object.trust_durations = 0;

383 - metalog_parser_object.private = &metalog_parser_state;

378 + PARSER_USER_OBJECT metalog_parser_object = {

379 + .enabled = cd.enabled,

380 + .host = ctx->rrdeng_ctx->host,

381 + .cd = &cd,

382 + .trust_durations = 0,

383 + .private = &metalog_parser_state

384 + };

385

386 PARSER *parser = parser_init(metalog_parser_object.host, &metalog_parser_object, NULL, PARSER_INPUT_SPLIT);

386 - if (unlikely(!parser)) {

387 - error("Failed to initialize metadata log parser.");

388 - failed_to_load = matched_files;

389 - goto after_failed_to_parse;

390 - }

387 parser_add_keyword(parser, PLUGINSD_KEYWORD_HOST, metalog_pluginsd_host);

388 parser_add_keyword(parser, PLUGINSD_KEYWORD_GUID, pluginsd_guid);

389 parser_add_keyword(parser, PLUGINSD_KEYWORD_CONTEXT, pluginsd_context);

@@ -428,10 +424,8 @@ static int scan_metalog_files(struct metalog_instance *ctx)

424 size_t count __maybe_unused = metalog_parser_object.count;

425

426 debug(D_METADATALOG, "Parsing count=%u", (unsigned)count);

431 -after_failed_to_parse:

427

428 freez(metalogfiles);

434 -

429 return matched_files;

430 }

431

database/engine/rrdengineapi.c

+5 -1

@@ -540,7 +540,7 @@ void rrdeng_load_metric_init(RRDDIM *rd, struct rrddim_query_handle *rrdimm_hand

540 rrdimm_handle->start_time = start_time;

541 rrdimm_handle->end_time = end_time;

542

543 - handle = calloc(1, sizeof(struct rrdeng_query_handle));

543 + handle = callocz(1, sizeof(struct rrdeng_query_handle));

544 handle->next_page_time = start_time;

545 handle->now = start_time;

546 handle->position = 0;

@@ -674,6 +674,10 @@ void rrdeng_load_metric_finalize(struct rrddim_query_handle *rrdimm_handle)

674 #endif

675 pg_cache_put(ctx, descr);

676 }

677 +

678 + // whatever is allocated at rrdeng_load_metric_init() should be freed here

679 + freez(handle);

680 + rrdimm_handle->handle = NULL;

681 }

682

683 time_t rrdeng_metric_latest_time(RRDDIM *rd)

database/rrdcalc.c

+9 -13

@@ -287,19 +287,15 @@ inline uint32_t rrdcalc_get_unique_id(RRDHOST *host, const char *chart, const ch

287 char *alarm_name_with_dim(char *name, size_t namelen, const char *dim, size_t dimlen) {

288 char *newname,*move;

289

290 - newname = malloc(namelen + dimlen + 2);

291 - if(newname) {

292 - move = newname;

293 - memcpy(move, name, namelen);

294 - move += namelen;

295 -

296 - *move++ = '_';

297 - memcpy(move, dim, dimlen);

298 - move += dimlen;

299 - *move = '\0';

300 - } else {

301 - newname = name;

302 - }

290 + newname = mallocz(namelen + dimlen + 2);

291 + move = newname;

292 + memcpy(move, name, namelen);

293 + move += namelen;

294 +

295 + *move++ = '_';

296 + memcpy(move, dim, dimlen);

297 + move += dimlen;

298 + *move = '\0';

299

300 return newname;

301 }

database/rrddim.c

+18 -18

@@ -169,31 +169,31 @@ static time_t rrddim_query_oldest_time(RRDDIM *rd) {

169

170 void rrdcalc_link_to_rrddim(RRDDIM *rd, RRDSET *st, RRDHOST *host) {

171 RRDCALC *rrdc;

172 +

173 for (rrdc = host->alarms_with_foreach; rrdc ; rrdc = rrdc->next) {

174 if (simple_pattern_matches(rrdc->spdim, rd->id) || simple_pattern_matches(rrdc->spdim, rd->name)) {

175 if (rrdc->hash_chart == st->hash_name || !strcmp(rrdc->chart, st->name) || !strcmp(rrdc->chart, st->id)) {

176 char *name = alarm_name_with_dim(rrdc->name, strlen(rrdc->name), rd->name, strlen(rd->name));

176 - if (name) {

177 - if(rrdcalc_exists(host, st->name, name, 0, 0)){

178 - freez(name);

179 - continue;

180 - }

177 + if(rrdcalc_exists(host, st->name, name, 0, 0)) {

178 + freez(name);

179 + continue;

180 + }

181

182 - netdata_rwlock_wrlock(&host->health_log.alarm_log_rwlock);

183 - RRDCALC *child = rrdcalc_create_from_rrdcalc(rrdc, host, name, rd->name);

184 - netdata_rwlock_unlock(&host->health_log.alarm_log_rwlock);

185 -

186 - if (child) {

187 - rrdcalc_add_to_host(host, child);

188 - RRDCALC *rdcmp = (RRDCALC *) avl_insert_lock(&(host)->alarms_idx_health_log,(avl_t *)child);

189 - if (rdcmp != child) {

190 - error("Cannot insert the alarm index ID %s",child->name);

191 - }

192 - } else {

193 - error("Cannot allocate a new alarm.");

194 - rrdc->foreachcounter--;

182 + netdata_rwlock_wrlock(&host->health_log.alarm_log_rwlock);

183 + RRDCALC *child = rrdcalc_create_from_rrdcalc(rrdc, host, name, rd->name);

184 + netdata_rwlock_unlock(&host->health_log.alarm_log_rwlock);

185 +

186 + if (child) {

187 + rrdcalc_add_to_host(host, child);

188 + RRDCALC *rdcmp = (RRDCALC *) avl_insert_lock(&(host)->alarms_idx_health_log,(avl_t *)child);

189 + if (rdcmp != child) {

190 + error("Cannot insert the alarm index ID %s",child->name);

191 }

192 }

193 + else {

194 + error("Cannot allocate a new alarm.");

195 + rrdc->foreachcounter--;

196 + }

197 }

198 }

199 }

parser/parser.c

+1 -11

@@ -33,20 +33,12 @@ PARSER *parser_init(RRDHOST *host, void *user, void *input, PARSER_INPUT_TYPE fl

33 PARSER *parser;

35 parser = callocz(1, sizeof(*parser));

36 -

37 - if (unlikely(!parser))

38 - return NULL;

39 -

36 parser->plugins_action = callocz(1, sizeof(PLUGINSD_ACTION));

41 - if (unlikely(!parser->plugins_action)) {

42 - freez(parser);

43 - return NULL;

44 - }

45 -

37 parser->user = user;

38 parser->input = input;

39 parser->flags = flags;

40 parser->host = host;

41 +

42 #ifdef ENABLE_HTTPS

43 parser->bytesleft = 0;

44 parser->readfrom = NULL;

@@ -181,9 +173,7 @@ void parser_destroy(PARSER *parser)

173 }

174

175 freez(parser->plugins_action);

184 -

176 freez(parser);

186 - return;

177 }

178

179

streaming/receiver.c

+28 -19

@@ -338,26 +338,31 @@ static char *receiver_next_line(struct receiver_state *r, int *pos) {

338 return NULL;

339 }

340

341 +static void streaming_parser_thread_cleanup(void *ptr) {

342 + PARSER *parser = (PARSER *)ptr;

343 + parser_destroy(parser);

344 +}

345 +

346 size_t streaming_parser(struct receiver_state *rpt, struct plugind *cd, FILE *fp) {

347 size_t result;

343 - PARSER_USER_OBJECT *user = callocz(1, sizeof(*user));

344 - user->enabled = cd->enabled;

345 - user->host = rpt->host;

346 - user->opaque = rpt;

347 - user->cd = cd;

348 - user->trust_durations = 0;

349 -

350 - PARSER *parser = parser_init(rpt->host, user, fp, PARSER_INPUT_SPLIT);

348 +

349 + PARSER_USER_OBJECT user = {

350 + .enabled = cd->enabled,

351 + .host = rpt->host,

352 + .opaque = rpt,

353 + .cd = cd,

354 + .trust_durations = 0

355 + };

356 +

357 + PARSER *parser = parser_init(rpt->host, &user, fp, PARSER_INPUT_SPLIT);

358 +

359 + // this keeps the parser with its current value

360 + // so, parser needs to be allocated before pushing it

361 + netdata_thread_cleanup_push(streaming_parser_thread_cleanup, parser);

362 +

363 parser_add_keyword(parser, "TIMESTAMP", streaming_timestamp);

364 parser_add_keyword(parser, "CLAIMED_ID", streaming_claimed_id);

365

354 - if (unlikely(!parser)) {

355 - error("Failed to initialize parser");

356 - cd->serial_failures++;

357 - freez(user);

358 - return 0;

359 - }

360 -

366 parser->plugins_action->begin_action = &pluginsd_begin_action;

367 parser->plugins_action->flush_action = &pluginsd_flush_action;

368 parser->plugins_action->end_action = &pluginsd_end_action;

@@ -371,12 +376,13 @@ size_t streaming_parser(struct receiver_state *rpt, struct plugind *cd, FILE *fp

376 parser->plugins_action->clabel_commit_action = &pluginsd_clabel_commit_action;

377 parser->plugins_action->clabel_action = &pluginsd_clabel_action;

378

374 - user->parser = parser;

379 + user.parser = parser;

380

381 #ifdef ENABLE_COMPRESSION

382 if (rpt->decompressor)

383 rpt->decompressor->reset(rpt->decompressor);

384 #endif

385 +

386 do{

387 if (receiver_read(rpt, fp))

388 break;

@@ -389,10 +395,13 @@ size_t streaming_parser(struct receiver_state *rpt, struct plugind *cd, FILE *fp

395 rpt->last_msg_t = now_realtime_sec();

396 }

397 while(!netdata_exit);

398 +

399 done:

393 - result= user->count;

394 - freez(user);

395 - parser_destroy(parser);

400 + result = user.count;

401 +

402 + // free parser with the pop function

403 + netdata_thread_cleanup_pop(1);

404 +

405 return result;

406 }

407

web/api/queries/des/des.c

+1 -1

@@ -70,7 +70,7 @@ static inline void set_beta(RRDR *r, struct grouping_des *g) {

70 }

72 void *grouping_create_des(RRDR *r) {

73 - struct grouping_des *g = (struct grouping_des *)malloc(sizeof(struct grouping_des));

73 + struct grouping_des *g = (struct grouping_des *)mallocz(sizeof(struct grouping_des));

74 set_alpha(r, g);

75 set_beta(r, g);

76 g->level = 0.0;