Skip to content

Commit 959abdd

Browse files
gkokolatosGeorgios KokolatosDimCitus
authored
Plug few isolated memleaks that showed up during profiling (#581)
Co-authored-by: Georgios Kokolatos <gkokolatos@pm.com> Co-authored-by: Dimitri Fontaine <dimitri@citusdata.com>
1 parent e5ba4f0 commit 959abdd

4 files changed

Lines changed: 90 additions & 0 deletions

File tree

‎src/bin/pg_autoctl/ipaddr.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,9 +537,11 @@ findHostnameLocalAddress(const char *hostname, char *localIpAddress, int size)
537537
size) == NULL)
538538
{
539539
log_warn("Failed to determine local ip address: %m");
540+
freeifaddrs(ifaddrList);
540541
return false;
541542
}
542543

544+
freeifaddrs(ifaddrList);
543545
return true;
544546
}
545547
}
@@ -565,15 +567,18 @@ findHostnameLocalAddress(const char *hostname, char *localIpAddress, int size)
565567
{
566568
/* check size >= INET6_ADDRSTRLEN */
567569
log_warn("Failed to determine local ip address: %m");
570+
freeifaddrs(ifaddrList);
568571
return false;
569572
}
570573

574+
freeifaddrs(ifaddrList);
571575
return true;
572576
}
573577
}
574578
}
575579
}
576580

581+
freeifaddrs(ifaddrList);
577582
freeaddrinfo(dns_lookup_addr);
578583
return false;
579584
}

‎src/bin/pg_autoctl/monitor.c‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,10 @@ monitor_print_nodes_as_json(Monitor *monitor, char *formation, int groupId)
416416
log_error("Failed to get the nodes from the monitor while running "
417417
"\"%s\" with formation %s and group %d",
418418
sql, formation, groupId);
419+
if (context.strVal)
420+
{
421+
free(context.strVal);
422+
}
419423
return false;
420424
}
421425

@@ -429,10 +433,15 @@ monitor_print_nodes_as_json(Monitor *monitor, char *formation, int groupId)
429433
"it returned an unexpected result. "
430434
"See previous line for details.",
431435
sql, formation, groupId);
436+
if (context.strVal)
437+
{
438+
free(context.strVal);
439+
}
432440
return false;
433441
}
434442

435443
fformat(stdout, "%s\n", context.strVal);
444+
free(context.strVal);
436445

437446
return true;
438447
}
@@ -559,6 +568,10 @@ monitor_print_other_nodes_as_json(Monitor *monitor,
559568
{
560569
log_error("Failed to get the other nodes from the monitor while running "
561570
"\"%s\" with node id %d", sql, myNodeId);
571+
if (context.strVal)
572+
{
573+
free(context.strVal);
574+
}
562575
return false;
563576
}
564577

@@ -571,10 +584,15 @@ monitor_print_other_nodes_as_json(Monitor *monitor,
571584
"\"%s\" with node id %d because it returned an "
572585
"unexpected result. See previous line for details.",
573586
sql, myNodeId);
587+
if (context.strVal)
588+
{
589+
free(context.strVal);
590+
}
574591
return false;
575592
}
576593

577594
fformat(stdout, "%s\n", context.strVal);
595+
free(context.strVal);
578596

579597
return true;
580598
}
@@ -1977,10 +1995,15 @@ monitor_print_state_as_json(Monitor *monitor, char *formation, int group)
19771995
{
19781996
log_error("Failed to parse current state from the monitor");
19791997
log_error("%s", context.strVal);
1998+
if (context.strVal)
1999+
{
2000+
free(context.strVal);
2001+
}
19802002
return false;
19812003
}
19822004

19832005
fformat(stdout, "%s\n", context.strVal);
2006+
free(context.strVal);
19842007

19852008
return true;
19862009
}
@@ -2142,10 +2165,15 @@ monitor_print_last_events_as_json(Monitor *monitor,
21422165
{
21432166
log_error("Failed to parse %d last events from the monitor", count);
21442167
log_error("%s", context.strVal);
2168+
if (context.strVal)
2169+
{
2170+
free(context.strVal);
2171+
}
21452172
return false;
21462173
}
21472174

21482175
fformat(stream, "%s\n", context.strVal);
2176+
free(context.strVal);
21492177

21502178
return true;
21512179
}
@@ -2379,17 +2407,26 @@ monitor_formation_uri(Monitor *monitor,
23792407
if (!context.parsedOk)
23802408
{
23812409
/* errors have already been logged */
2410+
if (context.strVal)
2411+
{
2412+
free(context.strVal);
2413+
}
23822414
return false;
23832415
}
23842416

23852417
if (context.strVal == NULL || strcmp(context.strVal, "") == 0)
23862418
{
23872419
log_error("Formation \"%s\" currently has no nodes in group 0",
23882420
formation);
2421+
if (context.strVal)
2422+
{
2423+
free(context.strVal);
2424+
}
23892425
return false;
23902426
}
23912427

23922428
strlcpy(connectionString, context.strVal, size);
2429+
free(context.strVal);
23932430

23942431
/* disconnect from PostgreSQL now */
23952432
pgsql_finish(&monitor->pgsql);
@@ -2490,13 +2527,18 @@ monitor_print_every_formation_uri_as_json(Monitor *monitor,
24902527
if (!context.parsedOk)
24912528
{
24922529
/* errors have already been logged */
2530+
if (context.strVal)
2531+
{
2532+
free(context.strVal);
2533+
}
24932534
return false;
24942535
}
24952536

24962537
/* disconnect from PostgreSQL now */
24972538
pgsql_finish(&monitor->pgsql);
24982539

24992540
fformat(stream, "%s\n", context.strVal);
2541+
free(context.strVal);
25002542

25012543
return true;
25022544
}
@@ -2752,10 +2794,15 @@ monitor_print_formation_settings_as_json(Monitor *monitor, char *formation)
27522794
{
27532795
log_error("Failed to parse formation settings from the monitor "
27542796
"for formation \"%s\"", formation);
2797+
if (context.strVal)
2798+
{
2799+
free(context.strVal);
2800+
}
27552801
return false;
27562802
}
27572803

27582804
fformat(stdout, "%s\n", context.strVal);
2805+
free(context.strVal);
27592806

27602807
return true;
27612808
}
@@ -2794,6 +2841,10 @@ monitor_synchronous_standby_names(Monitor *monitor,
27942841
log_error("Failed to get the synchronous_standby_names setting value "
27952842
" from the monitor for formation %s and group %d",
27962843
formation, groupId);
2844+
if (context.strVal)
2845+
{
2846+
free(context.strVal);
2847+
}
27972848
return false;
27982849
}
27992850

@@ -2803,10 +2854,15 @@ monitor_synchronous_standby_names(Monitor *monitor,
28032854
" from the monitor for formation %s and group %d,"
28042855
"see above for details",
28052856
formation, groupId);
2857+
if (context.strVal)
2858+
{
2859+
free(context.strVal);
2860+
}
28062861
return false;
28072862
}
28082863

28092864
strlcpy(synchronous_standby_names, context.strVal, size);
2865+
free(context.strVal);
28102866

28112867
return true;
28122868
}

‎src/bin/pg_autoctl/parsing.c‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ parse_state_notification_message(CurrentNodeState *nodeState,
267267
if (json_type(json) != JSONObject)
268268
{
269269
log_error("Failed to parse JSON notification message: \"%s\"", message);
270+
json_value_free(json);
270271
return false;
271272
}
272273

@@ -276,6 +277,7 @@ parse_state_notification_message(CurrentNodeState *nodeState,
276277
{
277278
log_error("Failed to parse JSOBJ notification state message: "
278279
"jsobj object type is not \"state\" as expected");
280+
json_value_free(json);
279281
return false;
280282
}
281283

@@ -285,6 +287,7 @@ parse_state_notification_message(CurrentNodeState *nodeState,
285287
{
286288
log_error("Failed to parse formation in JSON "
287289
"notification message \"%s\"", message);
290+
json_value_free(json);
288291
return false;
289292
}
290293
strlcpy(nodeState->formation, str, sizeof(nodeState->formation));
@@ -301,6 +304,7 @@ parse_state_notification_message(CurrentNodeState *nodeState,
301304
{
302305
log_error("Failed to parse node name in JSON "
303306
"notification message \"%s\"", message);
307+
json_value_free(json);
304308
return false;
305309
}
306310
strlcpy(nodeState->node.name, str, sizeof(nodeState->node.name));
@@ -311,6 +315,7 @@ parse_state_notification_message(CurrentNodeState *nodeState,
311315
{
312316
log_error("Failed to parse node host in JSON "
313317
"notification message \"%s\"", message);
318+
json_value_free(json);
314319
return false;
315320
}
316321
strlcpy(nodeState->node.host, str, sizeof(nodeState->node.host));
@@ -324,6 +329,7 @@ parse_state_notification_message(CurrentNodeState *nodeState,
324329
{
325330
log_error("Failed to parse reportedState in JSON "
326331
"notification message \"%s\"", message);
332+
json_value_free(json);
327333
return false;
328334
}
329335
nodeState->reportedState = NodeStateFromString(str);
@@ -334,6 +340,7 @@ parse_state_notification_message(CurrentNodeState *nodeState,
334340
{
335341
log_error("Failed to parse goalState in JSON "
336342
"notification message \"%s\"", message);
343+
json_value_free(json);
337344
return false;
338345
}
339346
nodeState->goalState = NodeStateFromString(str);
@@ -356,9 +363,11 @@ parse_state_notification_message(CurrentNodeState *nodeState,
356363
{
357364
log_error("Failed to parse health in JSON "
358365
"notification message \"%s\"", message);
366+
json_value_free(json);
359367
return false;
360368
}
361369

370+
json_value_free(json);
362371
return true;
363372
}
364373

@@ -761,6 +770,8 @@ parseNodesArray(const char *nodesJSON,
761770
"[{node_id:number, node_name:string, "
762771
"node_host:string, node_port:number, node_lsn:string, "
763772
"node_is_primary:boolean}, ...]");
773+
json_value_free(template);
774+
json_value_free(json);
764775
return false;
765776
}
766777

@@ -773,6 +784,8 @@ parseNodesArray(const char *nodesJSON,
773784
"%d nodes: pg_autoctl supports up to %d nodes",
774785
len,
775786
NODE_ARRAY_MAX_COUNT);
787+
json_value_free(template);
788+
json_value_free(json);
776789
return false;
777790
}
778791

@@ -812,6 +825,8 @@ parseNodesArray(const char *nodesJSON,
812825
if (!parseLSN(node->lsn, &lsn))
813826
{
814827
log_error("Failed to parse nodes array LSN value \"%s\"", node->lsn);
828+
json_value_free(template);
829+
json_value_free(json);
815830
return false;
816831
}
817832

@@ -825,13 +840,18 @@ parseNodesArray(const char *nodesJSON,
825840
{
826841
log_error("Failed to parse nodes array: more than one node "
827842
"is listed with \"node_is_primary\" true.");
843+
json_value_free(template);
844+
json_value_free(json);
828845
return false;
829846
}
830847
}
831848

832849
++nodesArrayIndex;
833850
}
834851

852+
json_value_free(template);
853+
json_value_free(json);
854+
835855
/* now ensure the array is sorted by nodeId */
836856
(void) pg_qsort(nodesArray->nodes,
837857
nodesArray->count,

‎src/bin/pg_autoctl/pgsetup.c‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,15 +460,20 @@ get_pgpid(PostgresSetup *pgSetup, bool pgIsNotRunningIsOk)
460460
{
461461
/* yeah, that happens (race condition, kind of) */
462462
log_debug("The PID file \"%s\" is empty", pidfile);
463+
free(contents);
463464
return false;
464465
}
465466
else if (splitLines(contents, lines, 1) != 1 ||
466467
!stringToInt(lines[0], &pid))
467468
{
468469
log_warn("Invalid data in PID file \"%s\"", pidfile);
470+
free(contents);
469471
return false;
470472
}
471473

474+
free(contents);
475+
contents = NULL;
476+
472477
/* postmaster PID (or negative of a standalone backend's PID) */
473478
if (pid < 0)
474479
{
@@ -764,6 +769,7 @@ pg_setup_get_local_connection_string(PostgresSetup *pgSetup,
764769
!get_env_copy("PG_REGRESS_SOCK_DIR", pg_regress_sock_dir, MAXPGPATH))
765770
{
766771
/* errors have already been logged */
772+
destroyPQExpBuffer(connStringBuffer);
767773
return false;
768774
}
769775

@@ -816,8 +822,11 @@ pg_setup_get_local_connection_string(PostgresSetup *pgSetup,
816822
"long, pg_autoctl only supports connection strings up to "
817823
" %d bytes",
818824
connStringBuffer->data, connStringBuffer->len, MAXCONNINFO);
825+
destroyPQExpBuffer(connStringBuffer);
826+
return false;
819827
}
820828

829+
destroyPQExpBuffer(connStringBuffer);
821830
return true;
822831
}
823832

0 commit comments

Comments
 (0)