Hi,
While testing the v27 patch set I hit a reproducible hang in the leader
apply worker, caused by an uninitialized read in the UPDATE path of
handle_dependency_on_change().
For an UPDATE, oldtup is filled by logicalrep_read_update() only when the
message carries an old tuple (the replica identity key changed). The
replica-identity check added in 0005 is guarded by that:
> if (has_oldtup)
> check_and_record_ri_dependency(relid, &oldtup, ...);
But the local-key check added in 0011 and the foreign-key check added in
0012 pass &oldtup with old_tuple=true unconditionally in the same case:
> check_and_record_local_key_dependency(relid, &oldtup, true, ...); /* 0011 */
> check_and_record_fkey_dependency(relid, &oldtup, true, ...); /* 0012 */
When the UPDATE does not change the replica identity, has_oldtup is false
and oldtup is never initialized, so both calls read uninitialized stack
memory.
Minimal repro (the subscriber has a local unique index on t(v); the update
touches a non-key column, so no old tuple is sent):
UPDATE t SET v = v + 1 WHERE id = :x;
Under gdb on the leader, oldtup.ncols comes out as garbage (I saw
-1475947488). The effect depends on the stack contents:
- a negative or small value: nothing visible;
- a large positive value: the leader spins in build_replica_identity_key()
(bms_is_member with values in the hundreds of millions), sits at 100% of
one core with the parallel workers idle, the publisher disconnects it on
wal_sender_timeout, and it relaunches and hangs again.
This is on a clean checkout, master 8e82d6610e72 with the v27 series. The
DELETE path is fine (logicalrep_read_delete always fills oldtup) and the
insert/new-tuple checks use newtup, so only the old-tuple checks in the
UPDATE case are affected. Guarding them with has_oldtup, as the 0005 RI
check already is, matches the existing pattern.
I have a self-contained repro script and can share it.
Regards,
Manu
