diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c index 4e32cfff50..30ddf94c95 100644 --- a/src/backend/access/transam/parallel.c +++ b/src/backend/access/transam/parallel.c @@ -37,7 +37,6 @@ #include "utils/guc.h" #include "utils/inval.h" #include "utils/memutils.h" -#include "utils/resowner.h" #include "utils/snapmgr.h" #include "utils/typcache.h" @@ -1220,16 +1219,19 @@ ParallelWorkerMain(Datum main_arg) Assert(ParallelWorkerNumber == -1); memcpy(&ParallelWorkerNumber, MyBgworkerEntry->bgw_extra, sizeof(int)); - /* Set up a memory context and resource owner. */ - Assert(CurrentResourceOwner == NULL); - CurrentResourceOwner = ResourceOwnerCreate(NULL, "parallel toplevel"); + /* Set up a memory context to work in, just for cleanliness. */ CurrentMemoryContext = AllocSetContextCreate(TopMemoryContext, "Parallel worker", ALLOCSET_DEFAULT_SIZES); /* - * Now that we have a resource owner, we can attach to the dynamic shared - * memory segment and read the table of contents. + * Attach to the dynamic shared memory segment for the parallel query, and + * find its table of contents. + * + * Note: at this point, we have not created any ResourceOwner in this + * process. This will result in our DSM mapping surviving until process + * exit, which is fine. If there were a ResourceOwner, it would acquire + * ownership of the mapping, but we have no need for that. */ seg = dsm_attach(DatumGetUInt32(main_arg)); if (seg == NULL) @@ -1393,7 +1395,7 @@ ParallelWorkerMain(Datum main_arg) /* Must exit parallel mode to pop active snapshot. */ ExitParallelMode(); - /* Must pop active snapshot so resowner.c doesn't complain. */ + /* Must pop active snapshot so snapmgr.c doesn't complain. */ PopActiveSnapshot(); /* Shut down the parallel-worker transaction. */ diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index bebe6405de..3ee6d5c467 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -6360,6 +6360,15 @@ StartupXLOG(void) bool fast_promoted = false; struct stat st; + /* + * We should have an aux process resource owner to use, and we should not + * be in a transaction that's installed some other resowner. + */ + Assert(AuxProcessResourceOwner != NULL); + Assert(CurrentResourceOwner == NULL || + CurrentResourceOwner == AuxProcessResourceOwner); + CurrentResourceOwner = AuxProcessResourceOwner; + /* * Verify XLOG status looks valid. */ @@ -8472,6 +8481,15 @@ GetNextXidAndEpoch(TransactionId *xid, uint32 *epoch) void ShutdownXLOG(int code, Datum arg) { + /* + * We should have an aux process resource owner to use, and we should not + * be in a transaction that's installed some other resowner. + */ + Assert(AuxProcessResourceOwner != NULL); + Assert(CurrentResourceOwner == NULL || + CurrentResourceOwner == AuxProcessResourceOwner); + CurrentResourceOwner = AuxProcessResourceOwner; + /* Don't be chatty in standalone mode */ ereport(IsPostmasterEnvironment ? LOG : NOTICE, (errmsg("shutting down"))); diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index 7e34bee63e..cdd71a9bc3 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -403,6 +403,13 @@ AuxiliaryProcessMain(int argc, char *argv[]) /* finish setting up bufmgr.c */ InitBufferPoolBackend(); + /* + * Auxiliary processes don't run transactions, but they may need a + * resource owner anyway to manage buffer pins acquired outside + * transactions (and, perhaps, other things in future). + */ + CreateAuxProcessResourceOwner(); + /* Initialize backend status information */ pgstat_initialize(); pgstat_bestart(); diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 78e4c85f5d..1d9cfc63d2 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -522,13 +522,9 @@ AutoVacLauncherMain(int argc, char *argv[]) pgstat_report_wait_end(); AbortBufferIO(); UnlockBuffers(); - if (CurrentResourceOwner) - { - ResourceOwnerRelease(CurrentResourceOwner, - RESOURCE_RELEASE_BEFORE_LOCKS, - false, true); - /* we needn't bother with the other ResourceOwnerRelease phases */ - } + /* this is probably dead code, but let's be safe: */ + if (AuxProcessResourceOwner) + ReleaseAuxProcessResources(false); AtEOXact_Buffers(false); AtEOXact_SMgr(); AtEOXact_Files(false); diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c index d5ce685e54..960d3de204 100644 --- a/src/backend/postmaster/bgwriter.c +++ b/src/backend/postmaster/bgwriter.c @@ -141,12 +141,6 @@ BackgroundWriterMain(void) /* We allow SIGQUIT (quickdie) at all times */ sigdelset(&BlockSig, SIGQUIT); - /* - * Create a resource owner to keep track of our resources (currently only - * buffer pins). - */ - CurrentResourceOwner = ResourceOwnerCreate(NULL, "Background Writer"); - /* * We just started, assume there has been either a shutdown or * end-of-recovery snapshot. @@ -191,11 +185,7 @@ BackgroundWriterMain(void) ConditionVariableCancelSleep(); AbortBufferIO(); UnlockBuffers(); - /* buffer pins are released here: */ - ResourceOwnerRelease(CurrentResourceOwner, - RESOURCE_RELEASE_BEFORE_LOCKS, - false, true); - /* we needn't bother with the other ResourceOwnerRelease phases */ + ReleaseAuxProcessResources(false); AtEOXact_Buffers(false); AtEOXact_SMgr(); AtEOXact_Files(false); diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 0950ada601..de1b22d045 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -231,12 +231,6 @@ CheckpointerMain(void) */ last_checkpoint_time = last_xlog_switch_time = (pg_time_t) time(NULL); - /* - * Create a resource owner to keep track of our resources (currently only - * buffer pins). - */ - CurrentResourceOwner = ResourceOwnerCreate(NULL, "Checkpointer"); - /* * Create a memory context that we will do all our work in. We do this so * that we can reset the context during error recovery and thereby avoid @@ -275,11 +269,7 @@ CheckpointerMain(void) pgstat_report_wait_end(); AbortBufferIO(); UnlockBuffers(); - /* buffer pins are released here: */ - ResourceOwnerRelease(CurrentResourceOwner, - RESOURCE_RELEASE_BEFORE_LOCKS, - false, true); - /* we needn't bother with the other ResourceOwnerRelease phases */ + ReleaseAuxProcessResources(false); AtEOXact_Buffers(false); AtEOXact_SMgr(); AtEOXact_Files(false); diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c index 688d5b5b80..eceed1bf88 100644 --- a/src/backend/postmaster/walwriter.c +++ b/src/backend/postmaster/walwriter.c @@ -129,12 +129,6 @@ WalWriterMain(void) /* We allow SIGQUIT (quickdie) at all times */ sigdelset(&BlockSig, SIGQUIT); - /* - * Create a resource owner to keep track of our resources (not clear that - * we need this, but may as well have one). - */ - CurrentResourceOwner = ResourceOwnerCreate(NULL, "Wal Writer"); - /* * Create a memory context that we will do all our work in. We do this so * that we can reset the context during error recovery and thereby avoid @@ -172,11 +166,7 @@ WalWriterMain(void) pgstat_report_wait_end(); AbortBufferIO(); UnlockBuffers(); - /* buffer pins are released here: */ - ResourceOwnerRelease(CurrentResourceOwner, - RESOURCE_RELEASE_BEFORE_LOCKS, - false, true); - /* we needn't bother with the other ResourceOwnerRelease phases */ + ReleaseAuxProcessResources(false); AtEOXact_Buffers(false); AtEOXact_SMgr(); AtEOXact_Files(false); diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c index 54c25f1f5b..45aae71a49 100644 --- a/src/backend/replication/logical/logicalfuncs.c +++ b/src/backend/replication/logical/logicalfuncs.c @@ -279,8 +279,6 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin */ startptr = MyReplicationSlot->data.restart_lsn; - CurrentResourceOwner = ResourceOwnerCreate(CurrentResourceOwner, "logical decoding"); - /* invalidate non-timetravel entries */ InvalidateSystemCaches(); @@ -320,6 +318,11 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin tuplestore_donestoring(tupstore); + /* + * Logical decoding could have clobbered CurrentResourceOwner during + * transaction management, so restore the executor's value. (This is + * a kluge, but it's not worth cleaning up right now.) + */ CurrentResourceOwner = old_resowner; /* diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 0d2b795e39..6ca6cdcacf 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -1595,6 +1595,11 @@ ApplyWorkerMain(Datum main_arg) pqsignal(SIGTERM, die); BackgroundWorkerUnblockSignals(); + /* + * We don't currently need any ResourceOwner in a walreceiver process, but + * if we did, we could call CreateAuxProcessResourceOwner here. + */ + /* Initialise stats to a sanish value */ MyLogicalRepWorker->last_send_time = MyLogicalRepWorker->last_recv_time = MyLogicalRepWorker->reply_time = GetCurrentTimestamp(); @@ -1602,10 +1607,6 @@ ApplyWorkerMain(Datum main_arg) /* Load the libpq-specific functions */ load_file("libpqwalreceiver", false); - Assert(CurrentResourceOwner == NULL); - CurrentResourceOwner = ResourceOwnerCreate(NULL, - "logical replication apply"); - /* Run as replica session replication role. */ SetConfigOption("session_replication_role", "replica", PGC_SUSET, PGC_S_OVERRIDE); diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c index 450f73759f..6d7474a976 100644 --- a/src/backend/replication/slotfuncs.c +++ b/src/backend/replication/slotfuncs.c @@ -365,9 +365,6 @@ pg_logical_replication_slot_advance(XLogRecPtr moveto) logical_read_local_xlog_page, NULL, NULL, NULL); - CurrentResourceOwner = ResourceOwnerCreate(CurrentResourceOwner, - "logical decoding"); - /* invalidate non-timetravel entries */ InvalidateSystemCaches(); @@ -402,6 +399,11 @@ pg_logical_replication_slot_advance(XLogRecPtr moveto) CHECK_FOR_INTERRUPTS(); } + /* + * Logical decoding could have clobbered CurrentResourceOwner during + * transaction management, so restore the executor's value. (This is + * a kluge, but it's not worth cleaning up right now.) + */ CurrentResourceOwner = old_resowner; if (ctx->reader->EndRecPtr != InvalidXLogRecPtr) diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index 987bb84683..7c292d8071 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -292,12 +292,6 @@ WalReceiverMain(void) if (WalReceiverFunctions == NULL) elog(ERROR, "libpqwalreceiver didn't initialize correctly"); - /* - * Create a resource owner to keep track of our resources (not clear that - * we need this, but may as well have one). - */ - CurrentResourceOwner = ResourceOwnerCreate(NULL, "Wal Receiver"); - /* Unblock signals (they were blocked when the postmaster forked us) */ PG_SETMASK(&UnBlockSig); diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index 76954831ce..e8f4f37e5c 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -90,7 +90,6 @@ #include "utils/pg_lsn.h" #include "utils/portal.h" #include "utils/ps_status.h" -#include "utils/resowner.h" #include "utils/timeout.h" #include "utils/timestamp.h" @@ -264,8 +263,10 @@ InitWalSender(void) /* Create a per-walsender data structure in shared memory */ InitWalSenderSlot(); - /* Set up resource owner */ - CurrentResourceOwner = ResourceOwnerCreate(NULL, "walsender top-level resource owner"); + /* + * We don't currently need any ResourceOwner in a walsender process, but + * if we did, we could call CreateAuxProcessResourceOwner here. + */ /* * Let postmaster know that we're a WAL sender. Once we've declared us as diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 09e0df290d..5ef6315d20 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -632,8 +632,19 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username, * We are either a bootstrap process or a standalone backend. Either * way, start up the XLOG machinery, and register to have it closed * down at exit. + * + * We don't yet have an aux-process resource owner, but StartupXLOG + * and ShutdownXLOG will need one. Hence, create said resource owner + * (and register a callback to clean it up after ShutdownXLOG runs). */ + CreateAuxProcessResourceOwner(); + StartupXLOG(); + /* Release (and warn about) any buffer pins leaked in StartupXLOG */ + ReleaseAuxProcessResources(true); + /* Reset CurrentResourceOwner to nothing for the moment */ + CurrentResourceOwner = NULL; + on_shmem_exit(ShutdownXLOG, 0); } diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c index bce021e100..211833da02 100644 --- a/src/backend/utils/resowner/resowner.c +++ b/src/backend/utils/resowner/resowner.c @@ -22,6 +22,7 @@ #include "access/hash.h" #include "jit/jit.h" +#include "storage/ipc.h" #include "storage/predicate.h" #include "storage/proc.h" #include "utils/memutils.h" @@ -140,6 +141,7 @@ typedef struct ResourceOwnerData ResourceOwner CurrentResourceOwner = NULL; ResourceOwner CurTransactionResourceOwner = NULL; ResourceOwner TopTransactionResourceOwner = NULL; +ResourceOwner AuxProcessResourceOwner = NULL; /* * List of add-on callbacks for resource releasing @@ -165,6 +167,7 @@ static void ResourceOwnerReleaseInternal(ResourceOwner owner, ResourceReleasePhase phase, bool isCommit, bool isTopLevel); +static void ReleaseAuxProcessResourcesCallback(int code, Datum arg); static void PrintRelCacheLeakWarning(Relation rel); static void PrintPlanCacheLeakWarning(CachedPlan *plan); static void PrintTupleDescLeakWarning(TupleDesc tupdesc); @@ -823,6 +826,60 @@ UnregisterResourceReleaseCallback(ResourceReleaseCallback callback, void *arg) } } +/* + * Establish an AuxProcessResourceOwner for the current process. + */ +void +CreateAuxProcessResourceOwner(void) +{ + Assert(AuxProcessResourceOwner == NULL); + Assert(CurrentResourceOwner == NULL); + AuxProcessResourceOwner = ResourceOwnerCreate(NULL, "AuxiliaryProcess"); + CurrentResourceOwner = AuxProcessResourceOwner; + + /* + * Register a shmem-exit callback for cleanup of aux-process resource + * owner. (This needs to run after, e.g., ShutdownXLOG.) + */ + on_shmem_exit(ReleaseAuxProcessResourcesCallback, 0); + +} + +/* + * Convenience routine to release all resources tracked in + * AuxProcessResourceOwner (but that resowner is not destroyed here). + * Warn about leaked resources if isCommit is true. + */ +void +ReleaseAuxProcessResources(bool isCommit) +{ + /* + * At this writing, the only thing that could actually get released is + * buffer pins; but we may as well do the full release protocol. + */ + ResourceOwnerRelease(AuxProcessResourceOwner, + RESOURCE_RELEASE_BEFORE_LOCKS, + isCommit, true); + ResourceOwnerRelease(AuxProcessResourceOwner, + RESOURCE_RELEASE_LOCKS, + isCommit, true); + ResourceOwnerRelease(AuxProcessResourceOwner, + RESOURCE_RELEASE_AFTER_LOCKS, + isCommit, true); +} + +/* + * Shmem-exit callback for the same. + * Warn about leaked resources if process exit code is zero (ie normal). + */ +static void +ReleaseAuxProcessResourcesCallback(int code, Datum arg) +{ + bool isCommit = (code == 0); + + ReleaseAuxProcessResources(isCommit); +} + /* * Make sure there is room for at least one more entry in a ResourceOwner's @@ -830,15 +887,12 @@ UnregisterResourceReleaseCallback(ResourceReleaseCallback callback, void *arg) * * This is separate from actually inserting an entry because if we run out * of memory, it's critical to do so *before* acquiring the resource. - * - * We allow the case owner == NULL because the bufmgr is sometimes invoked - * outside any transaction (for example, during WAL recovery). */ void ResourceOwnerEnlargeBuffers(ResourceOwner owner) { - if (owner == NULL) - return; + /* We used to allow pinning buffers without a resowner, but no more */ + Assert(owner != NULL); ResourceArrayEnlarge(&(owner->bufferarr)); } @@ -846,29 +900,19 @@ ResourceOwnerEnlargeBuffers(ResourceOwner owner) * Remember that a buffer pin is owned by a ResourceOwner * * Caller must have previously done ResourceOwnerEnlargeBuffers() - * - * We allow the case owner == NULL because the bufmgr is sometimes invoked - * outside any transaction (for example, during WAL recovery). */ void ResourceOwnerRememberBuffer(ResourceOwner owner, Buffer buffer) { - if (owner == NULL) - return; ResourceArrayAdd(&(owner->bufferarr), BufferGetDatum(buffer)); } /* * Forget that a buffer pin is owned by a ResourceOwner - * - * We allow the case owner == NULL because the bufmgr is sometimes invoked - * outside any transaction (for example, during WAL recovery). */ void ResourceOwnerForgetBuffer(ResourceOwner owner, Buffer buffer) { - if (owner == NULL) - return; if (!ResourceArrayRemove(&(owner->bufferarr), BufferGetDatum(buffer))) elog(ERROR, "buffer %d is not owned by resource owner %s", buffer, owner->name); diff --git a/src/include/utils/resowner.h b/src/include/utils/resowner.h index fe7f49119b..fa03942b6c 100644 --- a/src/include/utils/resowner.h +++ b/src/include/utils/resowner.h @@ -33,6 +33,7 @@ typedef struct ResourceOwnerData *ResourceOwner; extern PGDLLIMPORT ResourceOwner CurrentResourceOwner; extern PGDLLIMPORT ResourceOwner CurTransactionResourceOwner; extern PGDLLIMPORT ResourceOwner TopTransactionResourceOwner; +extern PGDLLIMPORT ResourceOwner AuxProcessResourceOwner; /* * Resource releasing is done in three phases: pre-locks, locks, and @@ -78,5 +79,7 @@ extern void RegisterResourceReleaseCallback(ResourceReleaseCallback callback, void *arg); extern void UnregisterResourceReleaseCallback(ResourceReleaseCallback callback, void *arg); +extern void CreateAuxProcessResourceOwner(void); +extern void ReleaseAuxProcessResources(bool isCommit); #endif /* RESOWNER_H */ diff --git a/src/test/modules/test_shm_mq/worker.c b/src/test/modules/test_shm_mq/worker.c index bcb992e1e4..4e23419c52 100644 --- a/src/test/modules/test_shm_mq/worker.c +++ b/src/test/modules/test_shm_mq/worker.c @@ -24,7 +24,6 @@ #include "storage/procarray.h" #include "storage/shm_mq.h" #include "storage/shm_toc.h" -#include "utils/resowner.h" #include "test_shm_mq.h" @@ -69,13 +68,16 @@ test_shm_mq_main(Datum main_arg) * Connect to the dynamic shared memory segment. * * The backend that registered this worker passed us the ID of a shared - * memory segment to which we must attach for further instructions. In - * order to attach to dynamic shared memory, we need a resource owner. - * Once we've mapped the segment in our address space, attach to the table - * of contents so we can locate the various data structures we'll need to + * memory segment to which we must attach for further instructions. Once + * we've mapped the segment in our address space, attach to the table of + * contents so we can locate the various data structures we'll need to * find within the segment. + * + * Note: at this point, we have not created any ResourceOwner in this + * process. This will result in our DSM mapping surviving until process + * exit, which is fine. If there were a ResourceOwner, it would acquire + * ownership of the mapping, but we have no need for that. */ - CurrentResourceOwner = ResourceOwnerCreate(NULL, "test_shm_mq worker"); seg = dsm_attach(DatumGetInt32(main_arg)); if (seg == NULL) ereport(ERROR, @@ -133,10 +135,8 @@ test_shm_mq_main(Datum main_arg) copy_messages(inqh, outqh); /* - * We're done. Explicitly detach the shared memory segment so that we - * don't get a resource leak warning at commit time. This will fire any - * on_dsm_detach callbacks we've registered, as well. Once that's done, - * we can go ahead and exit. + * We're done. For cleanliness, explicitly detach from the shared memory + * segment (that would happen anyway during process exit, though). */ dsm_detach(seg); proc_exit(1);