Skip to content

ggml: add null checks in ggml_vk_queue_command_pools_cleanup - #27353

Merged
0cc4m merged 4 commits into
ggml-org:masterfrom
jetsontan:master
Aug 19, 2026
Merged

ggml: add null checks in ggml_vk_queue_command_pools_cleanup#27353
0cc4m merged 4 commits into
ggml-org:masterfrom
jetsontan:master

Conversation

@jetsontan

@jetsontan jetsontan commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Overview

ggml_vk_queue_command_pools_cleanup() dereferences device->compute_queue and device->transfer_queue without null checks. These queues are std::unique_ptr members that can be nullptr during device teardown (they are explicitly reset in ~vk_device_struct()) or if the function is called before device initialization completes. In these cases the function crashes with a null pointer dereference.
The destructor ~vk_device_struct() already guards these same pointers with if (compute_queue) ..., but this cleanup function was missed.

Requirements

@jetsontan
jetsontan requested a review from a team as a code owner August 19, 2026 05:06
@github-actions github-actions Bot added Vulkan Issues specific to the Vulkan backend ggml changes relating to the ggml tensor library for machine learning labels Aug 19, 2026
@jetsontan jetsontan changed the title add null checks in ggml_vk_queue_command_pools_cleanup ggml: add null checks in ggml_vk_queue_command_pools_cleanup Aug 19, 2026
@ggml-gh-bot

This comment was marked as resolved.

@ggml-gh-bot ggml-gh-bot Bot added the draft PR will be changed to draft by github-actions bot label Aug 19, 2026
@github-actions
github-actions Bot marked this pull request as draft August 19, 2026 05:10
@github-actions github-actions Bot removed the draft PR will be changed to draft by github-actions bot label Aug 19, 2026
@jetsontan
jetsontan marked this pull request as ready for review August 19, 2026 05:16
@jetsontan
jetsontan requested a review from ggerganov as a code owner August 19, 2026 07:44
@jetsontan

jetsontan commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

@allozaur @CISC @ggerganov I'm new here. If I want to add a shell script or CMakeLists.txt so that users can call the Arm Mali Vulkan driver in Termux, where should I add it?

Comment thread AUTHORS Outdated
@CISC

CISC commented Aug 19, 2026

Copy link
Copy Markdown
Member

@allozaur @CISC @ggerganov I'm new here. If I want to add a shell script or CMakeLists.txt so that users can call the Arm Mali Vulkan driver in Termux, where should I add it?

What do you mean by call?

Co-authored-by: Sigbjørn Skjæret <sigbjorn.skjaeret@huggingface.co>
@jetsontan

Copy link
Copy Markdown
Contributor Author

@CISC
First, I apologize for any confusion in my comments — English is not my native language, so I have to rely on a translator, which may sometimes make my wording unclear.

The AUTHORS line has been removed as you suggested.

Regarding the Termux / Arm Mali Vulkan driver question — sorry for bringing it up here, it was off-topic for this PR. I’ll open a separate issue and describe it in more detail there.

If you have time, I’d really appreciate it if you could take another look at this small defensive fix when convenient. Thank you!

@CISC

CISC commented Aug 19, 2026

Copy link
Copy Markdown
Member

If you have time, I’d really appreciate it if you could take another look at this small defensive fix when convenient. Thank you!

@ggml-org/ggml-vulkan will attend to it.

Comment thread ggml/src/ggml-vulkan/ggml-vulkan.cpp Outdated
}
}


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary new line

Comment thread ggml/src/ggml-vulkan/ggml-vulkan.cpp Outdated
static constexpr uint32_t cleanup_frequency = 10;

if (device->compute_queue->cmd_pool.buffers_in_use() >= cleanup_frequency) {
// Guard against null queue pointers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment not needed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

finished

@jetsontan
jetsontan requested a review from 0cc4m August 19, 2026 12:17
@0cc4m
0cc4m merged commit b95502b into ggml-org:master Aug 19, 2026
27 of 29 checks passed
fewtarius added a commit to fewtarius/CachyLLama that referenced this pull request Aug 24, 2026
Brings 218 upstream commits into CachyLLama without losing any of our
features. Key carried-over changes from upstream:
- llama.cpp v0.2.0 / ggml v0.21.0 version bumps
- Vulkan FA MMQ fp32 scaling (ggml-org#27413), PAD_REFLECT_1D (ggml-org#26586), tiled
  transpose (ggml-org#26585), null checks in queue command pools cleanup (ggml-org#27353)
- ggml: rope_set_offset on multiple backends, recurrent state rollback
- Vulkan coopmat1 SHMEM_STRIDE_PAD/APPLY_SLM_A_RESHAPE for Intel Xe
- server: LLAMA_SERVER_SLOTS_N_DIFF (ggml-org#27600), /metrics during llama_decode
  (ggml-org#27041), index.html no-cache (ggml-org#27006), make-release workflow
- model: MiniMax-M1/Text01 (ggml-org#27018), Kimi-K3 (ggml-org#26185), BailingMoE3 (ggml-org#26608),
  GraniteSWA (ggml-org#25505), GLM-4.5-Air MTP, DSV4 tensor split (-sm tensor)
- ui: Chat Conversation Tabbed navigation, settings refactor
- common: --models-dir loading MTP assistant models (ggml-org#24431),
  --load-mode replacing --mmap (ggml-org#26934), json.h abstraction (ggml-org#27511)
- vendor: cpp-httplib 0.53.1, BoringSSL 0.20260813.0, vendor/hash

CachyLLama features preserved through conflict resolution:
- Persistent SSD-backed KV cache (3-tier hot/warm/cold + system prompt cache)
- Per-user isolation (user_id, per-user concurrency cap, slot affinity)
- MoE expert residency + co-activation tracking
- CachyLLama Vulkan Lightning Indexer (108/108 on Strix Halo) + DSV4
  hyper-connection fused ops + DSV4 sparse FA + coopmat shaders
- FA quant-KV dequant-once + f16 contiguize (with host-RAM safety gate)
- DFlash framework + Laguna-S-2.1 model support
- DFlash d2t reduced-vocab draft support (upstream merge)
- Context checkpoint ring buffer + SWA skip + memory budget scaling
- Stable-prefix LCP gate + prompt_stable_prefix_tokens param
- conv_hash conversation-boundary detection
- All CachyLLama Vulkan shaders (concat_transpose, lightning_indexer,
  mmid_row_lists, flash_attn_top_k, dequant_f16_transpose)
- common::host_available_ram() utility
- llama-moe-residency + llama-moe-coact modules

Manual conflict resolution touches: src/models/dflash.cpp (DFlash d2t +
aux_norm), src/llama-kv-cache-dsv4.cpp (state snapshot fix), src/llama-
memory-recurrent.cpp (rs_idx bounds check), src/llama-model-saver.cpp
(DSV4 compress_ratios + swiglu_clamp sizing), ggml/src/ggml-vulkan/
{ggml-vulkan.cpp,vulkan-shaders-gen.cpp,vulkan-shaders/dequant_q8_0.
comp,vulkan-shaders/flash_attn.comp,vulkan-shaders/copy_transpose_02.
comp} (CachyLLama shader registration + FA scratch gate), ggml/src/
ggml-cuda/mmvq.cu (RDNA3_5 + GB10 enum), gguf-py/gguf/constants.py
(DFlash ENC_AUX_NORM + D2T tensors), tests/{CMakeLists.txt,test-backend-
ops.cpp,test-llama-archs.cpp,test-recurrent-state-rollback.cpp}
(test additions), tools/{CMakeLists.txt,server/*} (server_batch embd
support + spec_is_replay + user_id routing + MCP servers + CORS), and
docs/{AGENTS.md,README.md} (kept CachyLLama branding).

Verified: full build succeeds, test-backend-ops Vulkan LIGHTNING_INDEXER +
FLASH_ATTN pass on Strix Halo.

Based on a re-merge from the 20260824 (pristine pre-merge) branch after
a previous agent's merge attempt produced an unbuildable state from
-X ours that wiped shader float-typing and broke the dequant_q8_0 +
flash_attn shaders with redefinition errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ggml changes relating to the ggml tensor library for machine learning Vulkan Issues specific to the Vulkan backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants