Skip to content

Commit

Permalink
Check for missing memory context switch back
Browse files Browse the repository at this point in the history
If memory context is switched temporarily and there is no switch back,
it will cause strange errors.

This adds a Coccinelle rule that checks for a case where the memory
context is saved in a temporary variable but this variable is not used
to switch back to the original memory context.

Co-authored-by: Fabrízio de Royes Mello <fabrizio@timescale.com>
  • Loading branch information
mkindahl and fabriziomello committed Mar 7, 2024
1 parent e5b5e43 commit c07decf
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 6 deletions.
21 changes: 21 additions & 0 deletions coccinelle/mcxt.cocci
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// find MemoryContextSwitchTo missing a context switch back
@ MemoryContextSwitch @
local idexpression oldctx;
position p;
@@
oldctx@p = MemoryContextSwitchTo(...);

@safelist@
local idexpression MemoryContextSwitch.oldctx;
position MemoryContextSwitch.p;
@@
oldctx@p = MemoryContextSwitchTo(...);
...
MemoryContextSwitchTo(oldctx)

@depends on !safelist@
expression oldctx;
position MemoryContextSwitch.p;
@@
+ /* MemoryContextSwitch missing context switch back */
oldctx@p = MemoryContextSwitchTo(...);
12 changes: 6 additions & 6 deletions src/telemetry/telemetry.c
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ add_errors_by_sqlerrcode(JsonbParseState *parse_state)
{
int res;
StringInfo command;
MemoryContext old_context = CurrentMemoryContext, spi_context;
MemoryContext orig_context = CurrentMemoryContext;

const char *command_string = "SELECT "
"job_type, jsonb_object_agg(sqlerrcode, count) "
Expand Down Expand Up @@ -391,11 +391,11 @@ add_errors_by_sqlerrcode(JsonbParseState *parse_state)
if (sqlerrs_jsonb == NULL)
continue;
/* the jsonb object cannot be created in the SPI context or it will be lost */
spi_context = MemoryContextSwitchTo(old_context);
MemoryContext spi_context = MemoryContextSwitchTo(orig_context);
add_errors_by_sqlerrcode_internal(parse_state,
TextDatumGetCString(record_jobtype),
sqlerrs_jsonb);
old_context = MemoryContextSwitchTo(spi_context);
MemoryContextSwitchTo(spi_context);
}

res = SPI_finish();
Expand Down Expand Up @@ -431,7 +431,7 @@ add_job_stats_by_job_type(JsonbParseState *parse_state)
{
StringInfo command;
int res;
MemoryContext old_context = CurrentMemoryContext, spi_context;
MemoryContext orig_context = CurrentMemoryContext;
SPITupleTable *tuptable = NULL;

const char *command_string =
Expand Down Expand Up @@ -511,7 +511,7 @@ add_job_stats_by_job_type(JsonbParseState *parse_state)
elog(ERROR, "null record field returned");
}

spi_context = MemoryContextSwitchTo(old_context);
MemoryContext spi_context = MemoryContextSwitchTo(orig_context);
TelemetryJobStats stats = { .total_runs = DatumGetInt64(total_runs),
.total_successes = DatumGetInt64(total_successes),
.total_failures = DatumGetInt64(total_failures),
Expand All @@ -522,7 +522,7 @@ add_job_stats_by_job_type(JsonbParseState *parse_state)
.total_duration_failures =
DatumGetIntervalP(total_duration_failures) };
add_job_stats_internal(parse_state, TextDatumGetCString(jobtype_datum), &stats);
old_context = MemoryContextSwitchTo(spi_context);
MemoryContextSwitchTo(spi_context);
}
res = SPI_finish();
Assert(res == SPI_OK_FINISH);
Expand Down

0 comments on commit c07decf

Please sign in to comment.