From fc0824a5580b00ce685d91964a8193527f62d908 Mon Sep 17 00:00:00 2001 From: Hou Zhijie Date: Fri, 28 Apr 2023 15:50:04 +0800 Subject: [PATCH 2/2] adjust the order of callback registration to avoid accessing invalid memory The callback function pa_shutdown(), which accesses MyLogicalRepWorker internally, was registered before initialization of MyLogicalRepWorker. As a result, if an ERROR occurs before initialization completes, pa_shutdown() will attempt to access invalid memory. Additionally, pa_shutdown() was registered before an exit callback function that resets MyLogicalRepWorker. If an ERROR occurs later, pa_shutdown() will be invoked after the reset, potentially leading to incorrect information. To fix this, reorder the registration process, so that pa_shutdown() is registered after the initialization and other exit callback function. While on it, add few comments atop pa_shutdown to enhance code readability. --- .../replication/logical/applyparallelworker.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/backend/replication/logical/applyparallelworker.c b/src/backend/replication/logical/applyparallelworker.c index c79765ce20..1bfb7ee06d 100644 --- a/src/backend/replication/logical/applyparallelworker.c +++ b/src/backend/replication/logical/applyparallelworker.c @@ -836,6 +836,9 @@ LogicalParallelApplyLoop(shm_mq_handle *mqh) * Make sure the leader apply worker tries to read from our error queue one more * time. This guards against the case where we exit uncleanly without sending * an ErrorResponse, for example because some code calls proc_exit directly. + * + * Also explicitly detach from dsm segment to fire on_dsm_detach callbacks. See + * ParallelWorkerShutdown for details. */ static void pa_shutdown(int code, Datum arg) @@ -892,8 +895,6 @@ ParallelApplyWorkerMain(Datum main_arg) (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("bad magic number in dynamic shared memory segment"))); - before_shmem_exit(pa_shutdown, PointerGetDatum(seg)); - /* Look up the shared information. */ shared = shm_toc_lookup(toc, PARALLEL_APPLY_KEY_SHARED, false); MyParallelShared = shared; @@ -912,6 +913,13 @@ ParallelApplyWorkerMain(Datum main_arg) */ logicalrep_worker_attach(worker_slot); + /* + * Register the callback after attaching to the worker slot to ensure it is + * invoked after MyLogicalRepWorker is initialized but before detaching + * from the slot. This ensures that MyLogicalRepWorker remains valid. + */ + before_shmem_exit(pa_shutdown, PointerGetDatum(seg)); + SpinLockAcquire(&MyParallelShared->mutex); MyParallelShared->logicalrep_worker_generation = MyLogicalRepWorker->generation; MyParallelShared->logicalrep_worker_slot_no = worker_slot; -- 2.30.0.windows.2