Fix race in parallel hash join batch cleanup, take II.
authorThomas Munro <[email protected]>
Tue, 21 Mar 2023 01:29:34 +0000 (14:29 +1300)
committerThomas Munro <[email protected]>
Tue, 21 Mar 2023 01:29:34 +0000 (14:29 +1300)
commit8d578b9b2e37a4d9d6f422ced5126acec62365a7
tree0d4aced1a942525eb8856c6c2d80a2942322b1fc
parentef719e7b32fc2ec9f78c2d9de099f7d75b61cd1b
Fix race in parallel hash join batch cleanup, take II.

With unlucky timing and parallel_leader_participation=off (not the
default), PHJ could attempt to access per-batch shared state just as it
was being freed.  There was code intended to prevent that by checking
for a cleared pointer, but it was racy.  Fix, by introducing an extra
barrier phase.  The new phase PHJ_BUILD_RUNNING means that it's safe to
access the per-batch state to find a batch to help with, and
PHJ_BUILD_DONE means that it is too late.  The last to detach will free
the array of per-batch state as before, but now it will also atomically
advance the phase, so that late attachers can avoid the hazard.  This
mirrors the way per-batch hash tables are freed (see phases
PHJ_BATCH_PROBING and PHJ_BATCH_DONE).

An earlier attempt to fix this (commit 3b8981b6, later reverted) missed
one special case.  When the inner side is empty (the "empty inner
optimization), the build barrier would only make it to
PHJ_BUILD_HASHING_INNER phase before workers attempted to detach from
the hashtable.  In that case, fast-forward the build barrier to
PHJ_BUILD_RUNNING before proceeding, so that our later assertions hold
and we can still negotiate who is cleaning up.

Revealed by build farm failures, where BarrierAttach() failed a sanity
check assertion, because the memory had been clobbered by dsa_free().
In non-assert builds, the result could be a segmentation fault.

Back-patch to all supported releases.

Author: Thomas Munro <[email protected]>
Author: Melanie Plageman <[email protected]>
Reported-by: Michael Paquier <[email protected]>
Reported-by: David Geier <[email protected]>
Tested-by: David Geier <[email protected]>
Discussion: https://postgr.es/m/20200929061142.GA29096%40paquier.xyz
src/backend/executor/nodeHash.c
src/backend/executor/nodeHashjoin.c
src/include/executor/hashjoin.h