diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index 65382c2..b8b73a4 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -1236,13 +1236,19 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn)
  * while accessing any catalogs. To enforce that, each decoding backend
  * has to call LogicalLockTransaction prior to any catalog access, and
  * then LogicalUnlockTransaction immediately after it. These functions
- * add/remove the decoding backend from a "decoding group" for a given
- * transaction. While aborting a prepared transaction, the backend will
- * wait for all current members of the decoding group to leave (see
- * LogicalDecodeRemoveTransaction).
+ * add the decoding backend into a "decoding group" for the transaction
+ * (on the first call), and then update a flag indicating whether the
+ * decoding backend may be accessing any catalogs.
  *
- * The function return true when it's safe to access catalogs, and
- * false when the transaction aborted (or is being aborted) in which
+ * While aborting a prepared transaction, the backend is made to wait
+ * for all current members of the decoding group that may be currently
+ * accessing catalogs (see LogicalDecodeRemoveTransaction). Once the
+ * transaction completes (applies to both abort and commit), the group
+ * is destroyed and is not needed anymore (we can check transaction
+ * status directly, instead).
+ *
+ * The function returns true when it's safe to access catalogs, and
+ * false when the transaction aborted (or is being aborted), in which
  * case the plugin should stop decoding it.
  *
  * The decoding backend joins the decoding group only when actually
@@ -1324,6 +1330,12 @@ LogicalLockTransaction(ReorderBufferTXN *txn)
 	/*
 	 * If we were able to add ourself, then Abort processing will
 	 * interlock with us.
+	 *
+	 * XXX There's a race condition here, I think. BecomeDecodeGroupMember
+	 * made us a member of the group, but the transaction might have
+	 * finished since then. In which case (decodeGroupLeader == NULL).
+	 * We need to set (decodeLocked = true) in BecomeDecodeGroupMember,
+	 * so that the leader waits for us.
 	 */
 	Assert(MyProc->decodeGroupLeader);
 
@@ -1333,6 +1345,9 @@ LogicalLockTransaction(ReorderBufferTXN *txn)
 	/*
 	 * Re-check if we were told to abort by the leader after taking
 	 * the above lock
+	 *
+	 * XXX It's not quite clear to me why we need the separate flag
+	 * in our process. Why not to simply check the leader's flag?
 	 */
 	if (MyProc->decodeAbortPending)
 	{
@@ -1410,7 +1425,12 @@ LogicalUnlockTransaction(ReorderBufferTXN *txn)
 	if (rbtxn_commit(txn))
 		return;
 
+	/*
+	 * We're guaranteed to still have a leader here, because were are
+	 * in locked mode, so the leader can't just disappear.
+	 */
 	Assert(MyProc->decodeGroupLeader);
+
 	leader_lwlock = LockHashPartitionLockByProc(MyProc->decodeGroupLeader);
 	LWLockAcquire(leader_lwlock, LW_SHARED);
 	if (MyProc->decodeAbortPending)
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index adb6ade..908eada 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -1396,6 +1396,7 @@ ReorderBufferCommitInternal(ReorderBufferTXN *txn,
 										 MAIN_FORKNUM));
 
 					/* Lock transaction before catalog access */
+					/* XXX Why no elog(LOG) here? */
 					if (!LogicalLockTransaction(txn))
 						break;
 
@@ -1443,6 +1444,7 @@ ReorderBufferCommitInternal(ReorderBufferTXN *txn,
 						 * transaction will be around for the duration of
 						 * the apply_change call below
 						 */
+						/* XXX Why no elog(LOG) here? */
 						if (!LogicalLockTransaction(txn))
 							break;
 						ReorderBufferToastReplace(rb, txn, relation, change);
@@ -1520,7 +1522,7 @@ ReorderBufferCommitInternal(ReorderBufferTXN *txn,
 					break;
 
 				case REORDER_BUFFER_CHANGE_MESSAGE:
-					/* XXX does rb->message need lock/unlock? */
+					/* XXX Why no elog(LOG) here? */
 					if (!LogicalLockTransaction(txn))
 						break;
 					rb->message(rb, txn, change->lsn, true,
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 2c002a2..3fc3a65 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -2004,7 +2004,6 @@ PGPROC *
 AssignDecodeGroupLeader(TransactionId xid)
 {
 	PGPROC *proc = NULL;
-	int			pid;
 	LWLock	   *leader_lwlock;
 
 	Assert(xid != InvalidTransactionId);
@@ -2015,9 +2014,7 @@ AssignDecodeGroupLeader(TransactionId xid)
 	 * If the transaction already completed, we can bail out.
 	 */
 	proc = BackendXidGetProc(xid);
-	if (proc)
-		pid = proc->pid;
-	else
+	if (!proc)
 		return NULL;
 
 	/*
@@ -2093,6 +2090,10 @@ AssignDecodeGroupLeader(TransactionId xid)
  * that, we require the caller to pass the PID of the intended PGPROC as
  * an interlock.  Returns true if we successfully join the intended lock
  * group, and false if not.
+ *
+ * XXX Not sure why are we passing-in the PID, considering we only deal
+ * with prepared transactions now, which means (pid==0). Shouldn't we
+ * use XID instead, for example?
  */
 bool
 BecomeDecodeGroupMember(PGPROC *leader, int pid, bool is_prepared)
@@ -2107,7 +2108,7 @@ BecomeDecodeGroupMember(PGPROC *leader, int pid, bool is_prepared)
 	Assert(MyProc->decodeGroupLeader == NULL);
 
 	/* PID must be valid OR this is a prepared transaction. */
-	Assert(pid != 0 || is_prepared);
+	Assert(((pid != 0) && !is_prepared) || ((pid == 0) && is_prepared));
 
 	/*
 	 * Get lock protecting the group fields.  Note LockHashPartitionLockByProc
@@ -2122,8 +2123,6 @@ BecomeDecodeGroupMember(PGPROC *leader, int pid, bool is_prepared)
 	/* Is this the leader we're looking for? */
 	if (leader->pid == pid && leader->decodeGroupLeader == leader)
 	{
-		if (is_prepared)
-			Assert(pid == 0);
 		/* is the leader going away? */
 		if (leader->decodeAbortPending)
 			ok = false;
@@ -2131,10 +2130,13 @@ BecomeDecodeGroupMember(PGPROC *leader, int pid, bool is_prepared)
 		{
 			/* OK, join the group */
 			ok = true;
+			/* XXX unfortunately this does not prevent the race in LockLogicalTransaction :-( */
+			MyProc->decodeLocked = true;
 			MyProc->decodeGroupLeader = leader;
 			dlist_push_tail(&leader->decodeGroupMembers, &MyProc->decodeGroupLink);
 		}
 	}
+	/* XXX seems unnecessary, considering the assert at the beginning */
 	else
 		MyProc->decodeGroupLeader = NULL;
 	LWLockRelease(leader_lwlock);
