Repository navigation
削除済: llama: embd_seq の解放を非同期出力コピー完了後に行う - #7
Merged
Merged
Conversation
`llama_context::decode()` で `embd_seq` を解放する前に、Metal バックエンドの
非同期出力コピーの完了を待つ。
Qwen3-VL-Embedding-2B を `/embeddings` の pooling `last` で使うと、画像入力を
含むリクエストが Metal (`-ngl 99`) で非決定的にヒープ破壊
(`BUG IN CLIENT OF LIBMALLOC`) を起こしクラッシュしていた。CPU では再現しない。
原因は `embd_seq` (LLAMA_POOLING_TYPE_{MEAN,CLS,LAST,RANK}
の出力を保持する `std::vector` 群) のライフサイクルにある。Metal の
`ggml_metal_get_tensor_async()` は出力先の生ポインタをゼロコピーで Metal
バッファに包んで blit をコマンドキューへ積むだけで、完了を待たずに返る
(`ggml-metal-context.m` の `waitUntilCompleted` 呼び出しは意図的にコメント
アウトされている)。一方 `decode()` は呼び出しのたびに冒頭で
`embd_seq.clear()` を行い、直前の呼び出しが書き込み先にしたベクタを即座に
解放する。画像トークン数が `n_batch` を超え `mtmd_helper_decode_image_chunk()`
が同期を挟まず `llama_decode()` を連続で呼ぶ経路 (Qwen-VL 系は 1 画像あたり
1024 トークン以上必要) では、直前の非同期コピーが完了する前にこの解放が起き、
GPU が解放済みで既に別用途に再利用されたメモリへ書き込んでヒープを破壊する。
テキストのみのリクエストは 1 回の decode() で完結するため競合が起きず、CPU
バックエンドの get_tensor_async は実質同期的なコピーのため再現しない。
関連: ishikawa/local-llm#71
## 設計
- `embd_seq` が空でない場合に限り `embd_seq.clear()` の直前で `synchronize()`
を呼ぶ。空の場合 (pooling を使わない通常の生成パスなど) は従来どおり何もしない
- 同期は `ggml_backend_sched_synchronize()` 経由で Metal の `cmd_buf_last` を
待つ既存の仕組みをそのまま使う。パイプライン並列時の同種の競合を避けている
既存コード (`process_ubatch()` の `pipeline_parallel` 分岐) と同じ考え方
- upstream 追従を妨げないよう、変更は `llama_context::decode()` の 1 箇所に閉じる
## 検証
Qwen3-VL-Embedding-2B (F16 + mmproj F16)、`--pooling last --ctx-size 8192
-ngl 99`。
- 修正前ビルドは画像埋め込みリクエストが 1 回目でほぼ確実にクラッシュすることを
確認 (`operator new` 内の `ggml_mem_ranges_init` 直前でヒープ破壊、5 回中 5
回再現。クラッシュフレームは無改造 upstream で採取したものと一致)
- 修正後ビルドは同一リクエストを 56 回連続実行しクラッシュなし (うち直近 30
回は同一プロセスを再起動せず連続実行)
<!-- prettier-ignore-start -->
| 比較 | cos 類似度 (最小) |
| --- | ---: |
| Metal (修正後) vs CPU、テキスト 5 件 | 0.999983 |
| Metal (修正後) vs CPU、画像 5 件 | 0.999549 |
| Metal (修正後) vs 修正前ベースライン、テキスト 5 件 | 0.999999 |
<!-- prettier-ignore-end -->
- 通常の chat completion (pooling なし、画像付きで `n_batch` を超える 1392
トークンの入力) で生成が正常に完了し、回帰なし
There was a problem hiding this comment.
Pull request overview
llama_context::decode() が次の decode 呼び出し冒頭で embd_seq を clear() する前に、バックエンド側の非同期出力コピー完了を待つことで、Metal などの非同期コピー経路で起き得る use-after-free / ヒープ破壊を防ぐことを目的とした変更です。
Changes:
embd_seqが空でない場合に限り、embd_seq.clear()の直前でsynchronize()を実行- 連続
decode()（特に mtmd の画像チャンク処理等）で、前回の非同期コピーが完了する前にバッファが解放される競合を回避
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
1770
to
1778
| // TODO: this clear of the buffer can easily be forgotten - need something better | ||
| // embeddings extracted via LLAMA_POOLING_TYPE_{MEAN,CLS,LAST,RANK} are copied into | ||
| // embd_seq asynchronously (see ggml_backend_tensor_get_async); a caller that issues | ||
| // back-to-back decode() calls without reading the output in between (e.g. mtmd image | ||
| // chunking) can otherwise free this buffer while the backend is still writing to it | ||
| if (!embd_seq.empty()) { | ||
| synchronize(); | ||
| } | ||
| embd_seq.clear(); |
Owner
Author
There was a problem hiding this comment.
指摘のとおり。ガードを t_compute_start_us の設定・n_queued_tokens の加算より前に移動し、synchronize() が常に直前のバッチの値だけを見るようにした (6498fc7)。
Comment on lines
+1771
to
+1774
| // embeddings extracted via LLAMA_POOLING_TYPE_{MEAN,CLS,LAST,RANK} are copied into | ||
| // embd_seq asynchronously (see ggml_backend_tensor_get_async); a caller that issues | ||
| // back-to-back decode() calls without reading the output in between (e.g. mtmd image | ||
| // chunking) can otherwise free this buffer while the backend is still writing to it |
Owner
Author
There was a problem hiding this comment.
指摘のとおり。decode() が memory 無しで encode() に委譲する経路で同じ use-after-free が残っていたため、encode() の embd_seq.clear() 直前にも同じガードを追加した (6498fc7)。
前コミットの `synchronize()` ガードの位置と適用範囲を見直す。 `decode()` 側は `t_compute_start_us` の設定・`n_queued_tokens` の加算より後に ガードを置いていたため、`synchronize()` が読む perf カウンタに今回バッチの 値が混入していた。`synchronize()` は呼び出し時点の `n_queued_tokens` / `t_compute_start_us` を前回分の計測として集計しリセットするため、この順序では 今回バッチのトークン数が前回分に混じり、今回の計算時間が計測から漏れる。 ガードをタイミング計測より前に移動し、`synchronize()` が常に「直前のバッチ」 の値だけを見るようにする。`embd_seq.clear()` / `output_swaps.clear()` の位置は 変更していない。 `encode()` も冒頭で `embd_seq.clear()` を行い、pooled embedding を `ggml_backend_tensor_get_async()` で `embd_seq` に書き込む。`decode()` は `memory` が無い場合に `encode()` へ委譲するため、前コミットで塞いだのと同じ use-after-free がここにも残っていた。`decode()` と同じガードを、同じく タイミング計測より前の位置に追加する。 ## 検証 Qwen3-VL-Embedding-2B (F16 + mmproj F16)、`--pooling last --ctx-size 8192 -ngl 99`。 - 画像埋め込みリクエストを 30 回連続実行しクラッシュなし - Metal vs CPU の cos 類似度: テキスト 2 件 (0.999998 / 0.999988)、画像 3 件 (最小 0.999549)、いずれも 0.9995 以上 - 前コミットの Metal 出力とのテキスト cos 類似度 1.000000 で、今回の変更が 出力値に影響しないことを確認
ishikawa
changed the title
ishikawa
changed the title
ishikawa
added a commit
that referenced
this pull request
Aug 9, 2026
#7 (1e08ebf) で追加した encode()/decode() の embd_seq 解放ガード (非同期出力コピー完了を待ってから embd_seq.clear() する) は、upstream 432d7ff (ggml-org#25676, "llama-context : sync pending async copies before clearing embd_seq") で同等のガードが独立に追加され、destructor 側の 同期も含めて fork 版より広い範囲を保護している。 コード上の guard 自体 (if (!embd_seq.empty()) { synchronize(); }) は upstream マージ後も機能的に同一のまま残っているため、revert は差分を 生まない。本コミットは fork 独自のコメント文言を upstream 432d7ff の 文言に揃えることで、fork 独自実装としての痕跡を除き、upstream 追従を 妨げないようにする。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
upstream で対応済みであり、差分は削除済みです。
llama_contextのdecode()/encode()でembd_seqを解放する前に、バックエンドの非同期出力コピーの完了を待つ。Qwen3-VL-Embedding-2B を
/embeddingsの poolinglastで使うと、画像入力を含むリクエストが Metal (-ngl 99) で非決定的にヒープ破壊 (BUG IN CLIENT OF LIBMALLOC) を起こしクラッシュしていた。CPU では再現しない。原因は
embd_seq(LLAMA_POOLING_TYPE_{MEAN,CLS,LAST,RANK} の出力を保持するstd::vector群) のライフサイクルにある。Metal のggml_metal_get_tensor_async()は出力先の生ポインタをゼロコピーで Metal バッファに包んで blit をコマンドキューへ積むだけで、完了を待たずに返る (ggml-metal-context.mのwaitUntilCompleted呼び出しは意図的にコメントアウトされている)。一方decode()/encode()は呼び出しのたびに冒頭でembd_seq.clear()を行い、直前の呼び出しが書き込み先にしたベクタを即座に解放する。画像トークン数がn_batchを超えmtmd_helper_decode_image_chunk()が同期を挟まずllama_decode()を連続で呼ぶ経路 (Qwen-VL 系は 1 画像あたり 1024 トークン以上必要) では、直前の非同期コピーが完了する前にこの解放が起き、GPU が解放済みで既に別用途に再利用されたメモリへ書き込んでヒープを破壊する。テキストのみのリクエストは 1 回の decode() で完結するため競合が起きず、CPU バックエンドの get_tensor_async は実質同期的なコピーのため再現しない。設計
embd_seqが空でない場合に限り、解放前にsynchronize()を呼ぶ。ガードはdecode()とencode()の両方に置く (decode()はmemoryがない場合encode()に委譲するため、片方だけでは同種の競合が残る)t_compute_start_us) とn_queued_tokensの更新より前に行う。synchronize()は直前バッチの perf 統計を確定してカウンタをリセットするため、後に置くと今回バッチのトークンが前回分として誤集計されるggml_backend_sched_synchronize()経由で Metal のcmd_buf_lastを待つ既存の仕組みをそのまま使う。パイプライン並列時の同種の競合を避けている既存コード (process_ubatch()のpipeline_parallel分岐) と同じ考え方embd_seqが空の場合 (pooling を使わない通常の生成パスなど) は従来どおり何もしないllama-context.cppの 2 箇所に閉じる検証
Qwen3-VL-Embedding-2B (F16 + mmproj F16)、
--pooling last --ctx-size 8192 -ngl 99。operator new内のggml_mem_ranges_init直前でヒープ破壊、5 回中 5 回再現。クラッシュフレームは無改造 upstream で採取したものと一致)n_batchを超える 1392 トークンの入力) で生成が正常に完了し、回帰なし