From 6150405679cf7a868761d13f2af3a61fc1d844b0 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Wed, 30 Sep 2026 04:33:16 +0000 Subject: [PATCH PG15 v2] 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 xlogreader.c 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 Reviewed-by: Sami Imseih Reviewed-by: Michael Paquier Reviewed-by: Chao Li Discussion: https://postgr.es/m/CALj2ACVwDuOXXDjj2cVdnTKoxsgTSLDin4XoL63AnM6aUgMQaA@mail.gmail.com Backpatch-through: 14 --- src/backend/access/transam/xlogreader.c | 116 ++++++++++++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 2 files changed, 117 insertions(+) diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 895f24ea69c..fe5e88cefbf 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -59,6 +59,34 @@ 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 WALRead() 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 WALRead() can tell whether a reader already has a + * callback and 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 XLogReaderResetCbState *find_reader_reset_cb(XLogReaderState *state); +static void xlogreader_close_segment(void *arg); +#endif + /* * Default size; large enough that typical users of XLogReader won't often need * to use the 'oversized' memory allocation code path. @@ -159,9 +187,71 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir, return state; } +#ifndef FRONTEND +/* + * Find the reset callback state for this reader, or NULL if it has none. + */ +static XLogReaderResetCbState * +find_reader_reset_cb(XLogReaderState *state) +{ + XLogReaderResetCbState *cbstate; + + for (cbstate = reader_reset_cbs; cbstate != NULL; cbstate = cbstate->next) + { + if (cbstate->reader == state) + return cbstate; + } + + return NULL; +} + +/* + * 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_close_segment(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. + */ + cbstate = find_reader_reset_cb(state); + if (cbstate != NULL) + cbstate->reader = NULL; +#endif + if (state->seg.ws_file != -1) state->routine.segment_close(state); @@ -1546,6 +1636,32 @@ WALRead(XLogReaderState *state, state->routine.segment_close(state); XLByteToSeg(recptr, nextSegNo, state->segcxt.ws_segsize); + +#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 the reader was allocated in as well. + */ + if (find_reader_reset_cb(state) == NULL) + { + MemoryContext readercxt = GetMemoryChunkContext(state); + XLogReaderResetCbState *cbstate; + + cbstate = MemoryContextAllocZero(readercxt, + sizeof(XLogReaderResetCbState)); + cbstate->cb.func = xlogreader_close_segment; + cbstate->cb.arg = cbstate; + cbstate->reader = state; + cbstate->next = reader_reset_cbs; + reader_reset_cbs = cbstate; + MemoryContextRegisterResetCallback(readercxt, &cbstate->cb); + } +#endif + state->routine.segment_open(state, nextSegNo, &tli); /* This shouldn't happen -- indicates a bug in segment_open */ diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index d0f927a9376..e5bb75045f2 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3025,6 +3025,7 @@ XLogPageReadResult XLogPrefetchStats XLogPrefetcher XLogPrefetcherFilter +XLogReaderResetCbState XLogReaderRoutine XLogReaderState XLogRecData -- 2.47.3