From 04a5e80f17fd9db6ee1f525701227286312ae8b4 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Mon, 21 Sep 2026 05:49:12 +0000 Subject: [PATCH PG18] Fix WAL segment file descriptor leak on WAL read errors. Previously, the WAL segment file that a WAL reader opens was closed only when the reader was freed. The descriptor is a plain kernel file descriptor, not a virtual file descriptor and not a transient file, so fd.c does not track it and no resource owner owns it. An error thrown while reading WAL therefore leaks it for the rest of the session. As a result, a few hundred failed calls in one session are enough to reach the descriptor limit, after which the backend cannot open any file at all, catalog files included. A leaked descriptor also pins a segment that has since been removed, so its space is not freed and the disk can fill up while pg_wal still looks small. The affected paths are pg_walinspect functions, logical decoding functions, the 2PC WAL read code, and the WAL summarizer. All of these except the WAL summarizer are reachable from SQL in simple ways. Fix this by registering a memory context reset callback on the context the reader is allocated in, which closes the segment file if that context is reset or deleted while the reader still holds it. XLogReaderFree() unregisters the callback before freeing the reader. Doing this in XLogReaderAllocate() covers every caller, present and future, instead of adding an error handler to each one. Note that PG18 and older cannot grow XLogReaderState, as it sits in a public header and its size must not change in a released branch, and they have no MemoryContextUnregisterResetCallback(). There the callback stays registered and its bookkeeping lives in a list private to xlogreader.c. Backpatch to all supported versions. Author: Bharath Rupireddy Discussion: https://postgr.es/m/ Backpatch-through: 14 --- src/backend/access/transam/xlogreader.c | 108 ++++++++++++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 2 files changed, 109 insertions(+) diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 96996bcb6ae..3ef87140d2d 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -58,6 +58,32 @@ static void WALOpenSegmentInit(WALOpenSegment *seg, WALSegmentContext *segcxt, /* size of the buffer allocated for error message. */ #define MAX_ERRORMSG_LEN 1000 +#ifndef FRONTEND +/* + * State for the reset callback that XLogReaderAllocate() registers on the + * memory context holding the reader. MemoryContextRegisterResetCallback() has + * no counterpart to unregister, so the callback stays on that context and + * XLogReaderFree() clears "reader" to leave it nothing to do. This is a + * separate allocation because it has to stay valid after the reader is freed. + */ +typedef struct XLogReaderResetCbState +{ + MemoryContextCallback cb; + XLogReaderState *reader; /* NULL once XLogReaderFree() has run */ + struct XLogReaderResetCbState *next; +} XLogReaderResetCbState; + +/* + * List of the above, so that XLogReaderFree() can find the entry for its + * reader. A pointer in XLogReaderState would do the same, but that would + * change the size of a struct exposed in a public header. Readers are + * allocated one or two at a time, so the list stays short. + */ +static XLogReaderResetCbState *reader_reset_cbs = NULL; + +static void xlogreader_reset_callback(void *arg); +#endif + /* * Default size; large enough that typical users of XLogReader won't often need * to use the 'oversized' memory allocation code path. @@ -108,6 +134,9 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir, XLogReaderRoutine *routine, void *private_data) { XLogReaderState *state; +#ifndef FRONTEND + XLogReaderResetCbState *cbstate; +#endif state = (XLogReaderState *) palloc_extended(sizeof(XLogReaderState), @@ -155,12 +184,91 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir, * enlarged if necessary. */ allocate_recordbuf(state, 0); + +#ifndef FRONTEND + + /* + * The WAL segment file is opened with BasicOpenFile(), so nothing but + * XLogReaderFree() ever closes it. An error thrown while reading WAL does + * not get that far, and the descriptor would then be leaked for the life + * of the process, so close it on a reset of the context we are allocated + * in as well. + */ + cbstate = (XLogReaderResetCbState *) + palloc_extended(sizeof(XLogReaderResetCbState), + MCXT_ALLOC_NO_OOM | MCXT_ALLOC_ZERO); + if (!cbstate) + { + pfree(state->errormsg_buf); + pfree(state->readRecordBuf); + pfree(state->readBuf); + pfree(state); + return NULL; + } + cbstate->cb.func = xlogreader_reset_callback; + cbstate->cb.arg = cbstate; + cbstate->reader = state; + cbstate->next = reader_reset_cbs; + reader_reset_cbs = cbstate; + MemoryContextRegisterResetCallback(CurrentMemoryContext, &cbstate->cb); +#endif + return state; } +#ifndef FRONTEND +/* + * Close the WAL segment file when the memory context holding the reader is + * reset or deleted, usually while an error is being handled. The reader is + * going away with that memory, so nothing can use the descriptor anymore. + * + * Reset callbacks run before the context's memory is freed, so the reader is + * still valid here. segment_close must not throw an error. + */ +static void +xlogreader_reset_callback(void *arg) +{ + XLogReaderResetCbState *cbstate = (XLogReaderResetCbState *) arg; + XLogReaderState *state = cbstate->reader; + XLogReaderResetCbState **link = &reader_reset_cbs; + + /* This entry's memory is about to go away, so take it off the list. */ + while (*link != NULL) + { + if (*link == cbstate) + { + *link = cbstate->next; + break; + } + link = &(*link)->next; + } + + if (state != NULL && state->seg.ws_file != -1) + state->routine.segment_close(state); +} +#endif + void XLogReaderFree(XLogReaderState *state) { +#ifndef FRONTEND + XLogReaderResetCbState *cbstate; + + /* + * The segment file is closed just below, so tell our reset callback it + * has nothing left to do. The entry stays on the list until the callback + * runs and removes it. + */ + for (cbstate = reader_reset_cbs; cbstate != NULL; cbstate = cbstate->next) + { + if (cbstate->reader == state) + { + cbstate->reader = NULL; + break; + } + } +#endif + if (state->seg.ws_file != -1) state->routine.segment_close(state); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 3ba0c7917bc..19b80fb1fc3 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3337,6 +3337,7 @@ XLogPageReadResult XLogPrefetchStats XLogPrefetcher XLogPrefetcherFilter +XLogReaderResetCbState XLogReaderRoutine XLogReaderState XLogRecData -- 2.47.3