Re: ENOSPC FailedAssertion("!(RefCountErrors == 0)"

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Re: ENOSPC FailedAssertion("!(RefCountErrors == 0)"
Дата
Msg-id 14892.1531872065@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Re: ENOSPC FailedAssertion("!(RefCountErrors == 0)"  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: ENOSPC FailedAssertion("!(RefCountErrors == 0)"  (Robert Haas <robertmhaas@gmail.com>)
Re: ENOSPC FailedAssertion("!(RefCountErrors == 0)"  (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>)
Список pgsql-hackers
I wrote:
>> So I said I didn't want to do extra work on this, but I am looking into
>> fixing it by having these aux process types run a ResourceOwner that can
>> be told to clean up any open buffer pins at exit.

> That turned out to be a larger can of worms than I'd anticipated, as it
> soon emerged that we'd acquired a whole lot of cargo-cult programming
> around ResourceOwners. ...
> I'm mostly pretty happy with this, but I think there are a couple of
> loose ends in logicalfuncs.c and slotfuncs.c: those are creating
> non-standalone ResourceOwners (children of whatever the active
> ResourceOwner is) and doing nothing much to clean them up.  That seems
> pretty bogus.

Further investigation showed that the part of that code that was
actually needed was not the creation of a child ResourceOwner, but rather
restoration of the old CurrentResourceOwner setting after exiting the
logical decoding loop.  Apparently we can come out of that with the
TopTransaction resowner being active.  This still seems a bit bogus;
maybe there should be a save-and-restore happening somewhere else?
But I'm not really excited about doing more than commenting it.

Also, most of the other random creations of ResourceOwners seem to just
not be necessary at all, even with the new rule that you must have a
resowner to acquire buffer pins.  So the attached revised patch just
gets rid of them, and improves some misleading/wrong comments on the
topic.  It'd still be easy to use CreateAuxProcessResourceOwner in any
process where we discover we need one, but I don't see the value in adding
useless overhead.

At this point I'm leaning to just applying this in HEAD and calling it
good.  The potential for assertion failures isn't relevant to production
builds, and my best guess at this point is that there isn't really any
other user-visible bug.  The resowners that were being created and not
adequately cleaned up all seem to have been dead code anyway (ie they'd
never acquire any resources).  We could have a problem if, say, the
bgwriter exited via the FATAL path while holding a pin, but I don't think
there's a reason for it to do that except in a database-wide shutdown,
where the consequences of a leaked pin seem pretty minimal.

Any objections?  Anyone want to do further review?

            regards, tom lane

diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 4e32cff..30ddf94 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 4049deb..5d01edd 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6356,6 +6356,15 @@ StartupXLOG(void)
     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.
      */
     if (ControlFile->state < DB_SHUTDOWNED ||
@@ -8467,6 +8476,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 7e34bee..cdd71a9 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 78e4c85..1d9cfc6 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 d5ce685..960d3de 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -142,12 +142,6 @@ BackgroundWriterMain(void)
     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 0950ada..de1b22d 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -232,12 +232,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
      * possible memory leaks.  Formerly this code just ran in
@@ -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 688d5b5..eceed1b 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -130,12 +130,6 @@ WalWriterMain(void)
     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
      * possible memory leaks.  Formerly this code just ran in
@@ -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 54c25f1..45aae71 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 0d2b795..6ca6cdc 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 450f737..6d7474a 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 987bb84..7c292d8 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 3a0106b..28f2435 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 09e0df2..5ef6315 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 bce021e..c708ebd 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,10 @@ 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;
     ResourceArrayEnlarge(&(owner->bufferarr));
 }

@@ -846,29 +898,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 fe7f491..fa03942 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 bcb992e..4e23419 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);

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Tomas Vondra
Дата:
Сообщение: Re: [HACKERS] PATCH: multivariate histograms and MCV lists
Следующее
От: Michael Paquier
Дата:
Сообщение: Re: Another usability issue with our TAP tests