Skip to content

Plug few isolated memleaks that showed up during profiling #581

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DimCitus merged 3 commits into from
Feb 5, 2021
Merged

Plug few isolated memleaks that showed up during profiling #581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2ab4c2a
Plug few isolated memleaks that showed up during profiling
Feb 4, 2021
2319a0f
Merge branch 'master' into pg_autoctl-memcheck
DimCitus Feb 5, 2021
656b1ba
Merge branch 'master' into pg_autoctl-memcheck
DimCitus Feb 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/bin/pg_autoctl/ipaddr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,9 +537,11 @@ findHostnameLocalAddress(const char *hostname, char *localIpAddress, int size)
size) == NULL)
{
log_warn("Failed to determine local ip address: %m");
freeifaddrs(ifaddrList);
return false;
}

freeifaddrs(ifaddrList);
return true;
}
}
Expand All @@ -565,15 +567,18 @@ findHostnameLocalAddress(const char *hostname, char *localIpAddress, int size)
{
/* check size >= INET6_ADDRSTRLEN */
log_warn("Failed to determine local ip address: %m");
freeifaddrs(ifaddrList);
return false;
}

freeifaddrs(ifaddrList);
return true;
}
}
}
}

freeifaddrs(ifaddrList);
freeaddrinfo(dns_lookup_addr);
return false;
}
Expand Down
56 changes: 56 additions & 0 deletions src/bin/pg_autoctl/monitor.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,10 @@ monitor_print_nodes_as_json(Monitor *monitor, char *formation, int groupId)
log_error("Failed to get the nodes from the monitor while running "
"\"%s\" with formation %s and group %d",
sql, formation, groupId);
if (context.strVal)
{
free(context.strVal);
Comment thread
DimCitus marked this conversation as resolved.
}
return false;
}

Expand All @@ -429,10 +433,15 @@ monitor_print_nodes_as_json(Monitor *monitor, char *formation, int groupId)
"it returned an unexpected result. "
"See previous line for details.",
sql, formation, groupId);
if (context.strVal)
{
free(context.strVal);
}
return false;
}

fformat(stdout, "%s\n", context.strVal);
free(context.strVal);

return true;
}
Expand Down Expand Up @@ -559,6 +568,10 @@ monitor_print_other_nodes_as_json(Monitor *monitor,
{
log_error("Failed to get the other nodes from the monitor while running "
"\"%s\" with node id %d", sql, myNodeId);
if (context.strVal)
{
free(context.strVal);
}
return false;
}

Expand All @@ -571,10 +584,15 @@ monitor_print_other_nodes_as_json(Monitor *monitor,
"\"%s\" with node id %d because it returned an "
"unexpected result. See previous line for details.",
sql, myNodeId);
if (context.strVal)
{
free(context.strVal);
}
return false;
}

fformat(stdout, "%s\n", context.strVal);
free(context.strVal);

return true;
}
Expand Down Expand Up @@ -1977,10 +1995,15 @@ monitor_print_state_as_json(Monitor *monitor, char *formation, int group)
{
log_error("Failed to parse current state from the monitor");
log_error("%s", context.strVal);
if (context.strVal)
{
free(context.strVal);
}
return false;
}

fformat(stdout, "%s\n", context.strVal);
free(context.strVal);

return true;
}
Expand Down Expand Up @@ -2142,10 +2165,15 @@ monitor_print_last_events_as_json(Monitor *monitor,
{
log_error("Failed to parse %d last events from the monitor", count);
log_error("%s", context.strVal);
if (context.strVal)
{
free(context.strVal);
}
return false;
}

fformat(stream, "%s\n", context.strVal);
free(context.strVal);

return true;
}
Expand Down Expand Up @@ -2379,17 +2407,26 @@ monitor_formation_uri(Monitor *monitor,
if (!context.parsedOk)
{
/* errors have already been logged */
if (context.strVal)
{
free(context.strVal);
}
return false;
}

if (context.strVal == NULL || strcmp(context.strVal, "") == 0)
{
log_error("Formation \"%s\" currently has no nodes in group 0",
formation);
if (context.strVal)
{
free(context.strVal);
}
return false;
}

strlcpy(connectionString, context.strVal, size);
free(context.strVal);

/* disconnect from PostgreSQL now */
pgsql_finish(&monitor->pgsql);
Expand Down Expand Up @@ -2490,13 +2527,18 @@ monitor_print_every_formation_uri_as_json(Monitor *monitor,
if (!context.parsedOk)
{
/* errors have already been logged */
if (context.strVal)
{
free(context.strVal);
}
return false;
}

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

fformat(stream, "%s\n", context.strVal);
free(context.strVal);

return true;
}
Expand Down Expand Up @@ -2752,10 +2794,15 @@ monitor_print_formation_settings_as_json(Monitor *monitor, char *formation)
{
log_error("Failed to parse formation settings from the monitor "
"for formation \"%s\"", formation);
if (context.strVal)
{
free(context.strVal);
}
return false;
}

fformat(stdout, "%s\n", context.strVal);
free(context.strVal);

return true;
}
Expand Down Expand Up @@ -2794,6 +2841,10 @@ monitor_synchronous_standby_names(Monitor *monitor,
log_error("Failed to get the synchronous_standby_names setting value "
" from the monitor for formation %s and group %d",
formation, groupId);
if (context.strVal)
{
free(context.strVal);
}
return false;
}

Expand All @@ -2803,10 +2854,15 @@ monitor_synchronous_standby_names(Monitor *monitor,
" from the monitor for formation %s and group %d,"
"see above for details",
formation, groupId);
if (context.strVal)
{
free(context.strVal);
}
return false;
}

strlcpy(synchronous_standby_names, context.strVal, size);
free(context.strVal);

return true;
}
Expand Down
20 changes: 20 additions & 0 deletions src/bin/pg_autoctl/parsing.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ parse_state_notification_message(CurrentNodeState *nodeState,
if (json_type(json) != JSONObject)
{
log_error("Failed to parse JSON notification message: \"%s\"", message);
json_value_free(json);
return false;
}

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

Expand All @@ -285,6 +287,7 @@ parse_state_notification_message(CurrentNodeState *nodeState,
{
log_error("Failed to parse formation in JSON "
"notification message \"%s\"", message);
json_value_free(json);
return false;
}
strlcpy(nodeState->formation, str, sizeof(nodeState->formation));
Expand All @@ -301,6 +304,7 @@ parse_state_notification_message(CurrentNodeState *nodeState,
{
log_error("Failed to parse node name in JSON "
"notification message \"%s\"", message);
json_value_free(json);
return false;
}
strlcpy(nodeState->node.name, str, sizeof(nodeState->node.name));
Expand All @@ -311,6 +315,7 @@ parse_state_notification_message(CurrentNodeState *nodeState,
{
log_error("Failed to parse node host in JSON "
"notification message \"%s\"", message);
json_value_free(json);
return false;
}
strlcpy(nodeState->node.host, str, sizeof(nodeState->node.host));
Expand All @@ -324,6 +329,7 @@ parse_state_notification_message(CurrentNodeState *nodeState,
{
log_error("Failed to parse reportedState in JSON "
"notification message \"%s\"", message);
json_value_free(json);
return false;
}
nodeState->reportedState = NodeStateFromString(str);
Expand All @@ -334,6 +340,7 @@ parse_state_notification_message(CurrentNodeState *nodeState,
{
log_error("Failed to parse goalState in JSON "
"notification message \"%s\"", message);
json_value_free(json);
return false;
}
nodeState->goalState = NodeStateFromString(str);
Expand All @@ -356,9 +363,11 @@ parse_state_notification_message(CurrentNodeState *nodeState,
{
log_error("Failed to parse health in JSON "
"notification message \"%s\"", message);
json_value_free(json);
return false;
}

json_value_free(json);
return true;
}

Expand Down Expand Up @@ -761,6 +770,8 @@ parseNodesArray(const char *nodesJSON,
"[{node_id:number, node_name:string, "
"node_host:string, node_port:number, node_lsn:string, "
"node_is_primary:boolean}, ...]");
json_value_free(template);
json_value_free(json);
return false;
}

Expand All @@ -773,6 +784,8 @@ parseNodesArray(const char *nodesJSON,
"%d nodes: pg_autoctl supports up to %d nodes",
len,
NODE_ARRAY_MAX_COUNT);
json_value_free(template);
json_value_free(json);
return false;
}

Expand Down Expand Up @@ -812,6 +825,8 @@ parseNodesArray(const char *nodesJSON,
if (!parseLSN(node->lsn, &lsn))
{
log_error("Failed to parse nodes array LSN value \"%s\"", node->lsn);
json_value_free(template);
json_value_free(json);
return false;
}

Expand All @@ -825,13 +840,18 @@ parseNodesArray(const char *nodesJSON,
{
log_error("Failed to parse nodes array: more than one node "
"is listed with \"node_is_primary\" true.");
json_value_free(template);
json_value_free(json);
return false;
}
}

++nodesArrayIndex;
}

json_value_free(template);
json_value_free(json);

/* now ensure the array is sorted by nodeId */
(void) pg_qsort(nodesArray->nodes,
nodesArray->count,
Expand Down
9 changes: 9 additions & 0 deletions src/bin/pg_autoctl/pgsetup.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,15 +460,20 @@ get_pgpid(PostgresSetup *pgSetup, bool pgIsNotRunningIsOk)
{
/* yeah, that happens (race condition, kind of) */
log_debug("The PID file \"%s\" is empty", pidfile);
free(contents);
return false;
}
else if (splitLines(contents, lines, 1) != 1 ||
!stringToInt(lines[0], &pid))
{
log_warn("Invalid data in PID file \"%s\"", pidfile);
free(contents);
return false;
}

free(contents);
contents = NULL;

/* postmaster PID (or negative of a standalone backend's PID) */
if (pid < 0)
{
Expand Down Expand Up @@ -764,6 +769,7 @@ pg_setup_get_local_connection_string(PostgresSetup *pgSetup,
!get_env_copy("PG_REGRESS_SOCK_DIR", pg_regress_sock_dir, MAXPGPATH))
{
/* errors have already been logged */
destroyPQExpBuffer(connStringBuffer);
return false;
}

Expand Down Expand Up @@ -816,8 +822,11 @@ pg_setup_get_local_connection_string(PostgresSetup *pgSetup,
"long, pg_autoctl only supports connection strings up to "
" %d bytes",
connStringBuffer->data, connStringBuffer->len, MAXCONNINFO);
destroyPQExpBuffer(connStringBuffer);
return false;
}

destroyPQExpBuffer(connStringBuffer);
return true;
}

Expand Down