Copilot commented on code in PR #2433:
URL: https://github.com/apache/age/pull/2433#discussion_r3381712279
##########
src/backend/utils/adt/age_global_graph.c:
##########
@@ -1125,17 +1108,17 @@ GRAPH_global_context *manage_GRAPH_global_contexts(char
*graph_name,
/* otherwise, we need to create one and possibly attach it */
new_ggctx = palloc0(sizeof(GRAPH_global_context));
- if (global_graph_contexts_container.contexts != NULL)
+ if (global_graph_contexts != NULL)
{
- new_ggctx->next = global_graph_contexts_container.contexts;
+ new_ggctx->next = global_graph_contexts;
}
else
{
new_ggctx->next = NULL;
}
/* set the global context variable */
- global_graph_contexts_container.contexts = new_ggctx;
+ global_graph_contexts = new_ggctx;
Review Comment:
`manage_GRAPH_global_contexts()` links `new_ggctx` into the global list
(`global_graph_contexts = new_ggctx;`) before the potentially error-throwing
build steps (`create_/load_/freeze_...`). If an `ereport(ERROR)` happens during
cache build (timeout/cancel/OOM) and the error is caught in a subtransaction
(e.g., PL/pgSQL `EXCEPTION`), the backend can continue with a partially
initialized `ggctx` still present and treated as valid (same snapshot/version),
leading to incorrect behavior or crashes on the next VLE call. Consider
constructing the context off-list and only linking it after a successful build,
and add `PG_TRY/PG_FINALLY` cleanup to free the partially built context (and
restore the previous `MemoryContext`) on error before rethrowing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
