Fix use-after-free on TRUNCATE of an EmbeddedRocksDB table being read - #112915

Fix round on the Alias truncate lock, all inside StorageAlias::truncate and
its test.

Take the target's exclusive lock with RWLockImpl::NO_QUERY rather than the
current query id. TRUNCATE TABLES ... LIKE truncates every matched table
concurrently on a thread pool sharing one query context, so two tasks
reaching the same target would hit RWLockImpl::getLock's same-query fast
path, which throws a logical error instead of waiting. An Alias is matched
by that loop because it does not override supportsTruncate. Two shapes are
reachable with a single statement: two aliases over one target, and an alias
next to its own target. Both abort a debug or sanitizer server before this
change and both now succeed. DatabaseMySQL.cpp:419 uses NO_QUERY for the
same reason.

Forward the interpreter's holder on the exempt MergeTree path instead of
passing an empty one. StorageReplicatedMergeTree::truncate consumes its
holder via release() to keep the replicated truncate asynchronous, so an
empty holder there would make that release a no-op and strand the alias's
exclusive lock for the whole asynchronous phase, up to the replication wait
timeout. The exempt branch is now byte-identical to the forwarding on
master, so that engine sees exactly what it saw before. The parameter is
used again, so its name is restored.

The test no longer relies on a fixed sleep to establish the race. It waits
via system.processes until a reader of the target is really running, then
requires TRUNCATE through the alias to report DEADLOCK_AVOIDED and to
succeed once the reader is gone. That oracle is independent of any
sanitizer: with the lock removed it reads 0 instead of 1 on a plain debug
build, and the server then aborts on the same free stack.

Renumbered to 04664 because 04657 was taken on master after this branch
diverged. Neither git nor check_gaps_in_tests_numbers reports a collision,
only gaps of 100 or more.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

TRUNCATE through an ENGINE = Alias took the exclusive lock on the alias only,
never on the target whose data it destroys, so a reader that reached the target
by another route kept using an EmbeddedRocksDB handle that TRUNCATE had already
closed and freed. That is fixed by locking the target, with MergeTree exempt
because it synchronizes truncation itself and the lock is heavyweight.

The exemption tested the pointer the catalog returned. A database created with
lazy_load_tables = 1 hands out a StorageTableProxy, which does not derive from
MergeTreeData, so a proxied MergeTree or ReplicatedMergeTree fell into the
non-exempt branch. For ReplicatedMergeTree that was a regression: it consumes
its holder via table_lock.release() to keep the truncate asynchronous, so
releasing a lock taken on the proxy instead left the alias's exclusive lock held
across the whole asynchronous phase. Unwrap StorageProxy before the decision and
use the unwrapped pointer only for it; truncate still goes through the target so
the proxy materializes and forwards, and the exempt branch still forwards the
interpreter's own holder unchanged.

Split the TRUNCATE TABLES ... LIKE coverage into its own test. Forcing several
pool tasks to want the same target lock at once needs the server-global PAUSEABLE
failpoint truncate_database_tables_pause, so that test is no-parallel and the
main one stays parallel-safe. Without it the tasks finish in microseconds and the
assertion can pass without ever exercising the overlap the NO_QUERY argument
exists to survive.

Wait for read_rows > 0 rather than mere presence in system.processes: the
ProcessList entry is published before the interpreter is built, hence before any
table lock is taken. Use INSERT ... SELECT rather than INSERT ... VALUES, since
the test runner redirects only stdout and stderr and a VALUES insert then blocks
on the inherited stdin until the test times out.

Report: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=109920&sha=9fc3b75335e01ca93d050a0336a98d9d85b98bf2&name_0=PR&name_1=BuzzHouse%20%28arm_asan_ubsan%29
Discovered on: ClickHouse#109920

Review round 4 found that both tests could pass without exercising what they
name. 04674 proved only that the TRUNCATE TABLES ... LIKE tasks overlapped at
the top of the per-table lambda, which is upstream of the target lock: if the
first task acquires and releases before the second asks, RWLockImpl::getLock's
same-query fast path is never reached and the test passes even with the query
id restored. And 04673's lazy-proxy cell asserted only the absence of
DEADLOCK_AVOIDED, which is also what a reader that never started, or a truncate
that failed for an unrelated reason, produces.

A PAUSEABLE_ONCE failpoint now parks one task while it owns the target's
exclusive lock, so another provably requests a lock that is already held. That
is the only state in which the query id matters, and both tests now assert the
state they need from the server rather than assuming it: the held/requesting
pair, the readiness of each reader, and that the exempt truncate actually
succeeded. Each assertion was mutation-tested; with the query id restored the
overlap test now fails 10 out of 10 runs.

Renumbered to 04677/04678: 04673 was claimed on master and by other in-flight
work while this branch was in review.

… target

The MergeTree exemption in StorageAlias::truncate is decided on the storage the
catalog entry really resolves to, but the walk followed only StorageProxy links.
StorageAlias is not a StorageProxy, so a chain of two aliases stopped at the
inner alias, the leaf was never reached, and the non-exempt branch ran.

Such a chain is constructible. The constructor rejects an alias whose target is
already an alias, but with lazy_load_tables the inner alias comes back from a
reload as an unloaded StorageTableProxy, whose getName() is "TableProxy", so the
guard does not fire. Two consequences: TRUNCATE over an exempt MergeTree leaf
started reporting DEADLOCK_AVOIDED under a concurrent reader where before this
branch it took no target lock at all, and with a ReplicatedMergeTree leaf the
inner alias forwarded its incoming holder into a callee whose first statement
releases it, so a lock this function owns would be released by a callee that
believes it owns the interpreter's holder.

The walk now follows both link kinds and is bounded by a visited set. Nothing
else changes: the lock is still taken on the catalog entry a reader locks, and
only the exemption decision uses the resolved leaf. No cycle is reachable today,
because the catalog's dependency graph rejects one, so that bound is defensive.

04677_alias_truncate_locks_target gains a cell for that chain. It fails on the
previous build with the truncate blocked and succeeds after this change, and
deleting only the alias-following half of the walk reddens that cell and nothing
else.

The exempt branch of StorageAlias::truncate forwards the interpreter's own
TableExclusiveLockHolder rather than a fresh one. That is deliberate:
StorageReplicatedMergeTree::truncate opens with table_lock.release() to keep the
replicated truncate asynchronous, and it is the only consumer of that holder type
in src/. Handing it an empty holder makes release() a no-op over a null drop_lock,
so the alias stays write-locked for the whole asynchronous phase and readers of the
alias get DEADLOCK_AVOIDED.

Nothing pinned that line. Both exemption cells of 04677 use plain MergeTree, whose
truncate leaves the holder parameter unnamed and therefore provably ignores it, so
the regression an earlier round fixed could return silently.

04686_alias_truncate_replicated_holder covers it with a ReplicatedMergeTree leaf
whose replication queue is stalled by SYSTEM STOP REPLICATION QUEUES: with no queue
entry executed the truncate's alter_sync = 1 wait cannot complete, so it parks
inside the asynchronous phase, and a read through the alias must still succeed. The
precondition is asserted from the queued DROP_RANGE in system.replication_queue
rather than from system.processes, because the ProcessList entry is published before
the interpreter is built and so long before the lock is released; without that row
the cell would also pass on a truncate that never started. Replacing the forwarded
holder with an empty one flips exactly its two oracle rows while every row of 04677
and 04678 stays byte-identical.

It needs zookeeper, no-shared-merge-tree and no-replicated-database, so it is a
separate file: adding those tags to 04677 would cost its other cells their coverage.
Cleanup restarts the queue from a trap, so an interrupted run cannot leave it
stopped for a later test.

Also note in the resolution comment that an alias chain does not need a lazy
database: the constructor guard inspects only an already-existing target and RENAME
does not re-validate, so renaming an alias onto a then-free target name forms one
with no proxy involved.

Every existing cell either truncates a raw storage, where the catalog entry and the
resolved leaf are the same object, or resolves to MergeTreeData and returns before the
lock is acquired. So none of them can distinguish locking the catalog entry from locking
the resolved leaf, and changing target_storage->lockExclusively to
unwrapped->lockExclusively left all three test files green.

That substitution is not harmless. A reader of a lazily loaded table locks the catalog
entry, while StorageProxy::read forwards to the nested storage without locking it, so a
lock on the leaf does not exclude that reader. Measured on the substituted binary, a
direct reader of a lazy EmbeddedRocksDB no longer blocks TRUNCATE through its alias, and
the truncate aborts inside rocksdb::DBImpl::Close while the reader's iterator is live,
which is the same lifetime violation this change exists to prevent.

The new cell wraps EmbeddedRocksDB in a lazy database, so the catalog hands out a proxy,
and requires a reader holding only the proxy's share lock to block the truncate. It fails
on the substituted binary with only the new rows flipping, and its companion row asserts
the truncate still succeeds once the reader is gone, so the block is real rather than a
permanently unavailable lock.

The readiness loop could fall through after its poll budget and truncate with no scan holding the
target lock, which would leave the first cell racing nothing while still reporting green.

Count the distinct scans observed reading and assert the count, so the prelude fails loudly instead
of silently losing its coverage. Accumulated across polls rather than sampled at one instant, because
a scan that already finished has also reached the target and requiring three at the same moment would
be its own source of flakiness.

Verified the assertion can fail: launching one scan instead of three reddens the test with
`scans reached target 3` -> `1`.

The lifetime of the target's data no longer depends on any lock taken here: the
preceding commit makes StorageEmbeddedRocksDB wait for its own readers, which covers the
direct path and the Buffer path too, neither of which goes through the alias.

Reverts these two files to their state before this branch, so StorageAlias::truncate
forwards as it does on master and the alias_truncate_target_lock_acquired failpoint goes
with the code that used it. The other non-MergeTree storages keep relying on the
interpreter's exclusive lock exactly as they do on master; IStorage::truncate's
documented contract is unchanged.

04677 previously asserted that TRUNCATE was BLOCKED by a live reader
(DEADLOCK_AVOIDED), which was a property of the alias-side lock the preceding commit
removes. It now asserts the stronger statement: with readers provably live
(read_rows > 0), TRUNCATE succeeds, the table is emptied only after they finish, and no
sanitizer report is produced. Both routes are covered, through the alias and directly on
the table, since the defect never depended on an alias.

The timeout cell asserts the original row count survives, not just the error code. A
design that nulled the handle and released the lock would report TIMEOUT_EXCEEDED while
silently leaving an empty table, so asserting the code alone would be a vacuous oracle.
It routes its TRUNCATE through the alias so the only thing that can delay the statement
is the drain rather than the interpreter's WRITE lock.

Readers are paced by wall clock over a bounded row count rather than by scanning the
whole table, which keeps the runtime independent of the build flavour.

04678 and 04686 are removed: 04678 existed only to cover RWLockImpl::NO_QUERY on the
target lock in TRUNCATE TABLES ... LIKE, and 04686 only the holder forwarding for
ReplicatedMergeTree targets. Both code paths are deleted in the preceding commit, so
neither test has a subject any more. This removes no coverage of behaviour that still
exists.

…a reader

The wait added for full scan leases converted the setting straight into a
duration and passed it to wait_for, so zero evaluated the predicate once and
gave up. Zero is documented as no locking timeout, and RWLockImpl::getLock
implements that by mapping it to time_point::max, so a TRUNCATE with the
documented "wait as long as it takes" value failed immediately whenever a read
was in progress. Since this change is what makes TRUNCATE on this engine read
the setting at all, the meaning of zero is this change's own contract.

The new test arm goes through the alias rather than the target. Truncating the
target directly takes an exclusive table lock first, and that lock reads the
same setting, so with zero it absorbs the whole wait and the branch under test
is never reached - a direct-route arm agrees on both builds and proves nothing.
Through the alias, only the target is left holding the reader, and the two
builds separate cleanly: waits about 9-19s and succeeds, against
TIMEOUT_EXCEEDED in under 600ms with the branch reverted.

Also assert what the concurrent-scan helper never did: that a waited-for read
actually finishes and returns every row. The readers' stdout went to /dev/null
and a bare wait discarded their exit statuses, so a build that drained the lease
correctly but broke the scan mid-iteration - the iterator's status turning bad
and generateFullScan throwing ROCKSDB_ERROR - still reported a successful
truncate and an empty table, and the test passed. sleepEachRow returns 0 per
row, so summing sleepEachRow(0.1) + 1 keeps the pacing while making the result
the row count. Verified with a mutant that throws mid-scan: only the new cells
move, from 3 to 0 and 1 to 0, while "truncate succeeded 1" and "rows after
truncate 0" stay put.

The rocksdb::Iterator forward declaration was unused; the lease class it was
added for is defined entirely in the .cpp.

TRUNCATE waited for the last full-scan lease while holding rocksdb_ptr_mx
exclusively. SharedMutex is writer-priority, so for the whole wait every other
user of the handle was fenced: both sinks, mutate, optimize, multiGet, the row
and byte estimators, and system.rocksdb.

That is worse than a stall. mutate re-takes the mutex shared inside its own
executor.pull loop, while its own read pipeline, and therefore its own lease,
is still alive. So

    TRUNCATE rdb_alias SETTINGS lock_acquire_timeout = 0
  concurrently with
    ALTER TABLE rdb DELETE WHERE <non-key predicate matching rows>

wedged both queries permanently: mutate parked in SharedMutex::lock_shared
waiting for the lock TRUNCATE held, TRUNCATE parked in condition_variable::wait
waiting for the lease mutate held. Nothing serialised them, because
StorageAlias::truncate forwards to the target without locking it.

Which lock is held where, after this change:

  - rocksdb_ptr_mx, exclusively: reading the lease count, Close(), nulling the
    pointer, wiping the directory, reopening. Never across a wait.
  - full_scan_leases_mx alone: the wait for the leases.
  - Order is one-way, rocksdb_ptr_mx then full_scan_leases_mx.

Reading the count under the exclusive lock is also what makes the close safe: a
lease can only be taken while holding rocksdb_ptr_mx shared, so observing zero
while holding it exclusively means none exists and none can appear before the
close. If a lease is outstanding the mutex is released and the wait retries
against one deadline for the whole call, so a reader arriving mid-wait cannot
extend the budget. The count is now an explicit counter rather than
use_count() on the drained pointer, which the exclusive holder no longer keeps.

The pointer is no longer moved out and restored, so a timed out TRUNCATE leaves
the table exactly as found by construction, and there is still no window in
which another query can observe a null handle.

The test grows a cell that runs a lease-holding mutation beside a waiting
TRUNCATE and asserts both complete, since every reader it had takes no mutex
and so could not tell this apart. Its TRUNCATE uses a bounded timeout on
purpose: with an unlimited one a regressed build reports by hanging until the
runner gives up, instead of failing the cell. The exact count() assertions are
pinned against optimize_trivial_approximate_count_query, which the runner
randomizes and under which the engine answers from a key estimate that drifts
once the same keys are rewritten.