Hi, On Fri, Sep 25, 2026 at 4:25 PM Bharath Rupireddy <[email protected]> wrote: > > > segment. I would imagine here that the sane move is to register a > > callback *iff* we open a segment. So, add a boolean flag in the state > > tracking if the reset callback is registered, and use > > GetMemoryChunkContext(state) to save the callback in the memory > > context of the xlogreader state, not the CurrentMemoryContext where > > the segment is opened. > > Agreed. A reader whose page_read callback reads everything from WAL > buffers, for example with WALReadFromBuffers(), may not call > segment_open at all, so it has no file to close, and registering a > callback for it at allocation time is wasted. Registering it lazily on > the first segment_open call avoids that. I will do it that way in the > next version, with the callback on the reader's own context. > > > Note that xlogreader.h declares a new variable that makes no sense in > > FRONTEND code. This needs an #ifdef. > > Ah, missed that. I will fix it. > > I will address the comments and post new patches soon.
Please find attached the v2 patches implementing lazy registration of the reset callback, only when a WAL segment is opened. v2-0001 is for HEAD and PG19. The nocfbot versions are for the back branches. -- Bharath Rupireddy Amazon Web Services: https://aws.amazon.com
From cc3c2147cc3892141c9e56c5ea09f028bfb9a20f Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Wed, 30 Sep 2026 03:15:23 +0000 Subject: [PATCH 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(). Backpatch to all supported versions. Author: Bharath Rupireddy <[email protected]> Reviewed-by: Sami Imseih <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: CALj2ACVwDuOXXDjj2cVdnTKoxsgTSLDin4XoL63AnM6aUgMQaA@mail.gmail.com">https://postgr.es/m/CALj2ACVwDuOXXDjj2cVdnTKoxsgTSLDin4XoL63AnM6aUgMQaA@mail.gmail.com Backpatch-through: 14 --- src/backend/access/transam/xlogreader.c | 55 +++++++++++++++++++++++++ src/include/access/xlogreader.h | 12 ++++++ 2 files changed, 67 insertions(+) diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 7db7c273b0c..a1ca68905d9 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -36,6 +36,7 @@ #ifndef FRONTEND #include "pgstat.h" #include "storage/bufmgr.h" +#include "utils/memutils.h" #include "utils/wait_event.h" #else #include "common/logging.h" @@ -55,6 +56,9 @@ static bool ValidXLogRecord(XLogReaderState *state, XLogRecord *record, static void ResetDecoder(XLogReaderState *state); static void WALOpenSegmentInit(WALOpenSegment *seg, WALSegmentContext *segcxt, int segsize, const char *waldir); +#ifndef FRONTEND +static void xlogreader_close_segment(void *arg); +#endif /* size of the buffer allocated for error message. */ #define MAX_ERRORMSG_LEN 1000 @@ -159,9 +163,40 @@ XLogReaderAllocate(int wal_segment_size, const char *waldir, 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_close_segment(void *arg) +{ + XLogReaderState *state = (XLogReaderState *) arg; + + if (state->seg.ws_file != -1) + state->routine.segment_close(state); +} +#endif + void XLogReaderFree(XLogReaderState *state) { +#ifndef FRONTEND + + /* + * Unregister the reset callback, which would otherwise be left pointing + * at freed memory. The context the reader was allocated in is the one it + * was registered on. + */ + if (state->reset_cb_registered) + MemoryContextUnregisterResetCallback(GetMemoryChunkContext(state), + &state->reset_cb); +#endif + if (state->seg.ws_file != -1) state->routine.segment_close(state); @@ -1597,6 +1632,26 @@ 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 (!state->reset_cb_registered) + { + state->reset_cb.func = xlogreader_close_segment; + state->reset_cb.arg = state; + MemoryContextRegisterResetCallback(GetMemoryChunkContext(state), + &state->reset_cb); + state->reset_cb_registered = true; + } +#endif + state->routine.segment_open(state, nextSegNo, &tli); /* This shouldn't happen -- indicates a bug in segment_open */ diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h index 4a9a687e879..b109005dccd 100644 --- a/src/include/access/xlogreader.h +++ b/src/include/access/xlogreader.h @@ -315,6 +315,18 @@ struct XLogReaderState * data. */ bool nonblocking; + +#ifndef FRONTEND + + /* + * Reset callback on the memory context holding this reader, which closes + * the open WAL segment file if that context goes away before + * XLogReaderFree() is reached. Registered on the first segment_open() + * call, see WALRead(). + */ + MemoryContextCallback reset_cb; + bool reset_cb_registered; +#endif }; /* -- 2.47.3
From 5e2aa592720930e989c9ba53b2d3afc057e6c73d Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Mon, 21 Sep 2026 05:49:12 +0000 Subject: [PATCH PG18 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 <[email protected]> Reviewed-by: Sami Imseih <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: CALj2ACVwDuOXXDjj2cVdnTKoxsgTSLDin4XoL63AnM6aUgMQaA@mail.gmail.com">https://postgr.es/m/CALj2ACVwDuOXXDjj2cVdnTKoxsgTSLDin4XoL63AnM6aUgMQaA@mail.gmail.com Backpatch-through: 14 --- src/backend/access/transam/xlogreader.c | 117 ++++++++++++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 2 files changed, 118 insertions(+) diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 96996bcb6ae..0be682a9ac9 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -36,6 +36,7 @@ #ifndef FRONTEND #include "pgstat.h" #include "storage/bufmgr.h" +#include "utils/memutils.h" #else #include "common/logging.h" #endif @@ -58,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. @@ -158,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); @@ -1549,6 +1640,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 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
From 3555168260cd2a772cfedc79fb8296884cdd1efb Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Wed, 30 Sep 2026 04:10:13 +0000 Subject: [PATCH PG17 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 <[email protected]> Reviewed-by: Sami Imseih <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: CALj2ACVwDuOXXDjj2cVdnTKoxsgTSLDin4XoL63AnM6aUgMQaA@mail.gmail.com">https://postgr.es/m/CALj2ACVwDuOXXDjj2cVdnTKoxsgTSLDin4XoL63AnM6aUgMQaA@mail.gmail.com Backpatch-through: 14 --- src/backend/access/transam/xlogreader.c | 117 ++++++++++++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 2 files changed, 118 insertions(+) diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 5bc659f89c3..f82767a75db 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -35,6 +35,7 @@ #ifndef FRONTEND #include "pgstat.h" +#include "utils/memutils.h" #else #include "common/logging.h" #endif @@ -57,6 +58,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. @@ -157,9 +186,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); @@ -1545,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 d5314ce86d4..3473229864d 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3197,6 +3197,7 @@ XLogPageReadResult XLogPrefetchStats XLogPrefetcher XLogPrefetcherFilter +XLogReaderResetCbState XLogReaderRoutine XLogReaderState XLogRecData -- 2.47.3
From 3ab42d5d165508f9060e3eda4cd0cf2f5a53ee5c Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Wed, 30 Sep 2026 04:14:09 +0000 Subject: [PATCH PG16 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 <[email protected]> Reviewed-by: Sami Imseih <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: CALj2ACVwDuOXXDjj2cVdnTKoxsgTSLDin4XoL63AnM6aUgMQaA@mail.gmail.com">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 d3fbfd6bca8..48671501369 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); @@ -1550,6 +1640,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 725c29c4e2c..7413b28dfe7 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3081,6 +3081,7 @@ XLogPageReadResult XLogPrefetchStats XLogPrefetcher XLogPrefetcherFilter +XLogReaderResetCbState XLogReaderRoutine XLogReaderState XLogRecData -- 2.47.3
From 6150405679cf7a868761d13f2af3a61fc1d844b0 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> 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 <[email protected]> Reviewed-by: Sami Imseih <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: CALj2ACVwDuOXXDjj2cVdnTKoxsgTSLDin4XoL63AnM6aUgMQaA@mail.gmail.com">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
From 5e68c090a2e4856c5c032063ed149cc2d6730fb7 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Wed, 30 Sep 2026 04:37:17 +0000 Subject: [PATCH PG14 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 <[email protected]> Reviewed-by: Sami Imseih <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Reviewed-by: Chao Li <[email protected]> Discussion: CALj2ACVwDuOXXDjj2cVdnTKoxsgTSLDin4XoL63AnM6aUgMQaA@mail.gmail.com">https://postgr.es/m/CALj2ACVwDuOXXDjj2cVdnTKoxsgTSLDin4XoL63AnM6aUgMQaA@mail.gmail.com Backpatch-through: 14 --- src/backend/access/transam/xlogreader.c | 115 ++++++++++++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 2 files changed, 116 insertions(+) diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 3c7a548bcf1..a60a0c9e68d 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -50,6 +50,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 + /* * Construct a string in state->errormsg_buf explaining what's wrong with * the current record being read. @@ -128,10 +156,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) { int block_id; +#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); @@ -1126,6 +1215,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 19a4ada1642..faa4ec05314 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2923,6 +2923,7 @@ XLogPageHeader XLogPageHeaderData XLogPageReadCB XLogPageReadPrivate +XLogReaderResetCbState XLogReaderRoutine XLogReaderState XLogRecData -- 2.47.3
