mirror of
https://github.com/ClickHouse/ClickHouse.git
synced 2024-11-22 23:52:03 +00:00
Fix possible crash for hedged requests
Previously, it was possible for hedged requests to continue choosing replica even after the query had been cancelled (RemoteQueryExecutor::tryCancel()), and not only this does not make sense, but could also lead to a crash, due to use-after-free of current_thread (ThreadStatus), since fiber had been created on a different thread (thread for query pipeline), but will be destroyed from another thread (that calls QueryPipeline dtor), and the query pipeline's thread could be already destroyed by that time (especially under threads pressure). v0: IConnection::cancelAsync() v2: remove it, since the query is sent in a deferred manner for hedged requests, so that said that modifying HedgedConnections::sendCancel() should be enough Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
This commit is contained in:
parent
923caa52ba
commit
ff6d1d09d6
@ -255,6 +255,17 @@ void HedgedConnections::sendCancel()
|
||||
if (!sent_query || cancelled)
|
||||
throw Exception(ErrorCodes::LOGICAL_ERROR, "Cannot cancel. Either no query sent or already cancelled.");
|
||||
|
||||
/// All hedged connections should be stopped, since otherwise before the
|
||||
/// HedgedConnectionsFactory will be destroyed (that will happen from
|
||||
/// QueryPipeline dtor) they could still do some work.
|
||||
/// And not only this does not make sense, but it also could lead to
|
||||
/// use-after-free of the current_thread, since the thread from which they
|
||||
/// had been created differs from the thread where the dtor of
|
||||
/// QueryPipeline will be called and the initial thread could be already
|
||||
/// destroyed (especially when the system is under pressure).
|
||||
if (hedged_connections_factory.hasEventsInProcess())
|
||||
hedged_connections_factory.stopChoosingReplicas();
|
||||
|
||||
cancelled = true;
|
||||
|
||||
for (auto & offset_status : offset_states)
|
||||
|
Loading…
Reference in New Issue
Block a user