[protectione055]

What does this PR do?

GpDropTempTables() updates slotid under the per-slot lock, while
SharedSnapshotAdd() and SharedSnapshotLookup() scan slot IDs under
SharedSnapshotLock. These accesses use different locks and can run
concurrently.

Acquire SharedSnapshotLock in exclusive mode before the existing
per-slot lock when updating slotid during a session reset. This
serializes the update with directory scans while preserving protection
for per-slot readers. Release the locks in reverse order.

Type of Change

• Bug fix (non-breaking change)
• New feature (non-breaking change)
• Breaking change (fix or feature with breaking changes)
• Documentation update

Test Plan

• Unit tests added/updated
• Integration tests added/updated
• Passed make installcheck
• Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:
Adds an exclusive directory-lock acquisition when a session reset
updates its shared snapshot slot ID.

Checklist

• Followed contribution guide
• Added/updated documentation
• Reviewed code for security implications
• This PR contains AI-assisted code generation
• Requested review from cloudberry committers

[yjhjstz]

Looks good to me. GpDropTempTables() was updating slotid under only the per-slot slotLock, while SharedSnapshotAdd() and SharedSnapshotLookup() scan slotid under SharedSnapshotLock. Since the two sides used different locks, a scan could race with the update. Taking SharedSnapshotLock first, then slotLock, fixes that.

I checked sharedsnapshot.c and found no path that holds slotLock and then waits on SharedSnapshotLock (the error path in getSharedSnapshot releases slotLock first), so the lock order is consistent and shouldn't deadlock. The extra exclusive lock only applies on session reset, which is rare.

Two optional suggestions:

1. Add a comment near the slotLock field noting that the directory lock must be taken first.
2. Mention how the race was found (reproduction or code reading).

[github-actions]

Hi, @protectione055 welcome!🎊 Thanks for taking the effort to make our project better! 🙌 Keep making such awesome contributions!

[protectione055]

Thanks for the review and suggestions.

Two optional suggestions:

1. Add a comment near the slotLock field noting that the directory lock must be taken first.

I’ve added a comment next to slotLock documenting that SharedSnapshotLock should be acquired first when both locks are needed.

2. Mention how the race was found (reproduction or code reading).

I found the locking mismatch while reading the code and haven’t constructed a test case that reproduces a failure. In theory, if a concurrent scan observed a partial slotid value, it could mistake the slot for another session’s slot or an unused slot. I haven’t verified that such a read can occur on a supported platform.