From ba12c0f84008e244efabe06bc32fa99b504fa0af Mon Sep 17 00:00:00 2001 From: Mason Sharp Date: Tue, 11 Aug 2026 20:17:55 -0700 Subject: [PATCH 1/2] Copy the error message out of ErrorContext before taking it queue_item_note_error() called CopyErrorData() from a PG_CATCH() body, which is the one place it may not run: errstart() switches to ErrorContext and an error longjmps out without switching back, so the copy lands in the context FlushErrorState() is about to reset. An assert-enabled build aborts outright on the assertion CopyErrorData() carries for this; a release build frees the copy under itself and gets away with it only because the message is taken first. Switch away for the duration of the copy. test/t/004 goes from killing the postmaster before its first assertion to passing all seven, and its fifth - that the recorded reason is the error itself - is reached for the first time. --- src/worker.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/worker.c b/src/worker.c index b1c0eb0..87d5c67 100644 --- a/src/worker.c +++ b/src/worker.c @@ -242,17 +242,30 @@ queue_item_begin(int64 queue_id, int attempts, int max_attempts) static void queue_item_note_error(void) { - ErrorData *edata; + ErrorData *edata; + MemoryContext oldcontext; if (failed_item_queue_id < 0) return; + /* + * CopyErrorData() copies into the current context and asserts it is not + * ErrorContext, which is exactly where a PG_CATCH() body starts: errstart() + * switches there and an error longjmps out without switching back. Left + * alone this aborts an assert-enabled build outright, and on a release + * build the copy is freed under it by FlushErrorState(). The copy is only + * needed until the message has been taken, so any other context will do. + */ + oldcontext = MemoryContextSwitchTo(TopMemoryContext); + edata = CopyErrorData(); if (edata->message != NULL) strlcpy(failed_item_error, edata->message, sizeof(failed_item_error)); FreeErrorData(edata); + + MemoryContextSwitchTo(oldcontext); } /* From 7c046eaea1b005adf54a5e5bd6a34b1576ea301c Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 12 Aug 2026 10:33:00 +0100 Subject: [PATCH 2/2] Correct the comment on the ErrorContext switch The comment named errstart() where it is errfinish() that switches to ErrorContext and then deliberately leaves CurrentMemoryContext set there when it re-throws, and it claimed FlushErrorState() frees the copy out from under us, when FreeErrorData() frees it explicitly a few lines earlier. The real cost on a release build is milder and worth stating accurately: the copy eats into the space ErrorContext reserves so that reporting an error can never itself fail, which is precisely what the assertion in CopyErrorData() is protecting. --- src/worker.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/worker.c b/src/worker.c index 87d5c67..107f828 100644 --- a/src/worker.c +++ b/src/worker.c @@ -250,11 +250,13 @@ queue_item_note_error(void) /* * CopyErrorData() copies into the current context and asserts it is not - * ErrorContext, which is exactly where a PG_CATCH() body starts: errstart() - * switches there and an error longjmps out without switching back. Left - * alone this aborts an assert-enabled build outright, and on a release - * build the copy is freed under it by FlushErrorState(). The copy is only - * needed until the message has been taken, so any other context will do. + * ErrorContext, which is exactly where a PG_CATCH() body starts: + * errfinish() switches there and deliberately leaves it set that way when + * it re-throws. Left alone this aborts an assert-enabled build outright; + * on a release build it instead eats into the space ErrorContext reserves + * so that reporting an error can never itself fail, which is what the + * assertion protects. The copy is only needed until the message has been + * taken, so any other context will do. */ oldcontext = MemoryContextSwitchTo(TopMemoryContext);