Fix segfault in dictionary internal query

[mstetsyuk]

Linked issues

• fixes https://github.com/ClickHouse/clickhouse-core-incidents/issues/1464

Changelog category (leave one):

• Critical Bug Fix (crash, data loss, RBAC)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fix null pointer dereference segfault when loading dictionaries during server shutdown. Context::getUserDefinedSQLObjectsStorage (dereferences user_defined_sql_objects_storage) is called by dictionary threads concurrently with the main thread calling Context::shutdown (sets user_defined_sql_objects_storage to null). We need to make sure we disable future updates in the dictionaries loader, kill the currently running dictionary queries and join the dictionary loading threads - all before running Context::shutdown. Similar to what we do with normal queries.

Documentation entry for user-facing changes

• Documentation is written (mandatory for new features)

[clickhouse-gh]

Workflow [PR], commit [a765f3b]

Summary: ❌

job_name	test_name	status	info	comment
Integration tests (amd_llvm_coverage, 5/5)		failure
	test_server_reload/test.py::test_change_listen_host	FAIL	cidb
Finish Workflow		failure
	python3 ./ci/jobs/scripts/workflow_hooks/new_tests_check.py	failure

---

AI Review

Summary

This PR fixes a shutdown-time null dereference by making Context::shutdown stop dictionary periodic updates, cancel in-flight dictionary-related queries via ProcessList::killAllQueries, and wait for dictionary loading threads via ExternalLoader::joinLoadingThreads before tearing down context-owned objects. The change is focused, consistent with the stated incident, and I did not find additional correctness, safety, or compatibility issues in the final diff.

ClickHouse Rules

Item	Status	Notes
Deletion logging	➖
Serialization versioning	➖
Core-area scrutiny	✅
No test removal	✅
Experimental gate	➖
No magic constants	✅
Backward compatibility	✅
SettingsChangesHistory.cpp	➖
PR metadata quality	✅
Safe rollout	✅
Compilation time	✅

Final Verdict

• Status: ✅ Approve

[mstetsyuk]

Confirmed against core dump:

(lldb) p ((DB::Context*)0x00007d0226169400)->shared->shutdown_called
warning: `this' is not accessible (substituting 0). Couldn't load 'this' because its value couldn't be evaluated
(std::atomic<bool>) {
  Value = true
}
(lldb) p ((DB::Context*)0x00007d0226169400)->shared->user_defined_sql_objects_storage
warning: `this' is not accessible (substituting 0). Couldn't load 'this' because its value couldn't be evaluated
(std::unique_ptr<DB::IUserDefinedSQLObjectsStorage>) nullptr {
  pointer = nullptr
}
(lldb)

[azat]

I don't think we need this, we have safeExit in case there are alive connections

And it looks like a hack

[mstetsyuk]

I don't think we need this, we have safeExit in case there are alive connections

This is needed for dictionary queries.

We need to (1) disable future updates, (2) kill all the currently running queries, and (3) join the threads.

If we skip (2), (3) can take a long time to finish.

[azat]

I see, then let's add a comment and also I guess we need to do this unconditionally, since between previous KillAllQueries and this one dictionary can start reloading?

By the way, is it hard to make proper query termination from joinLoadingThreads?

[mstetsyuk]

By the way, is it hard to make proper query termination from joinLoadingThreads?

It is quite hard. There are arguably too many layers of abstraction between ExternalDictionariesLoader and the code that runs the selects.

I guess we need to do this unconditionally, since between previous KillAllQueries and this one dictionary can start reloading?

Yes, there can be new queries. My initial idea was to still diligently respect the shutdown_wait_unfinished_queries setting, but it probably doesn't matter, so I removed the check.

[clickhouse-gh]

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	84.20%	84.10%	-0.10%
Functions	24.50%	24.30%	-0.20%
Branches	76.70%	76.60%	-0.10%

Changed lines: 92.59% (25/27) · Uncovered code

Full report · Diff report