diff --git a/crisp_audio/src/audio_tower.cpp b/crisp_audio/src/audio_tower.cpp index 80dc72e2e..aa584fd3e 100644 --- a/crisp_audio/src/audio_tower.cpp +++ b/crisp_audio/src/audio_tower.cpp @@ -684,8 +684,12 @@ void crisp_audio_free(struct crisp_audio_context* ctx) { #endif if (ctx->sched) ggml_backend_sched_free(ctx->sched); - if (ctx->model_buf) - ggml_backend_buffer_free(ctx->model_buf); + // model_buf came from core_gguf::load_weights, so on a device advertising + // buffer_from_host_ptr it is a view onto a host mmap the backend does not + // own. ggml_backend_buffer_free() alone would leave the weight file mapped + // for the life of the process. The release entry point takes the loader's + // side-map entry and unmaps; it no-ops on a null handle and nulls ours. + core_gguf::release_weight_buffer(ctx->model_buf); if (ctx->model_ctx) ggml_free(ctx->model_ctx); if (ctx->backend_cpu) diff --git a/crisp_lid/src/lid_cld3.cpp b/crisp_lid/src/lid_cld3.cpp index 098e92611..019cab175 100644 --- a/crisp_lid/src/lid_cld3.cpp +++ b/crisp_lid/src/lid_cld3.cpp @@ -804,7 +804,7 @@ extern "C" void lid_cld3_free(lid_cld3_context* ctx) { if (!ctx) return; if (ctx->buf) - ggml_backend_buffer_free(ctx->buf); + core_gguf::release_weight_buffer(ctx->buf); if (ctx->gctx) ggml_free(ctx->gctx); delete ctx; diff --git a/crisp_punc/src/fireredpunc.cpp b/crisp_punc/src/fireredpunc.cpp index 469565e11..bc2a5e0f9 100644 --- a/crisp_punc/src/fireredpunc.cpp +++ b/crisp_punc/src/fireredpunc.cpp @@ -905,7 +905,7 @@ void fireredpunc_free(fireredpunc_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->buf) - ggml_backend_buffer_free(ctx->buf); + core_gguf::release_weight_buffer(ctx->buf); if (ctx->w_ctx) ggml_free(ctx->w_ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/crisp_punc/src/pcs.cpp b/crisp_punc/src/pcs.cpp index 985741de1..4491c45fa 100644 --- a/crisp_punc/src/pcs.cpp +++ b/crisp_punc/src/pcs.cpp @@ -1021,7 +1021,7 @@ void pcs_free(pcs_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->buf) - ggml_backend_buffer_free(ctx->buf); + core_gguf::release_weight_buffer(ctx->buf); if (ctx->w_ctx) ggml_free(ctx->w_ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/ark_asr.cpp b/src/ark_asr.cpp index 10c20e615..daeae23f2 100644 --- a/src/ark_asr.cpp +++ b/src/ark_asr.cpp @@ -1179,9 +1179,9 @@ extern "C" void ark_asr_free(struct ark_asr_context* ctx) { if (ctx->kv_ctx) ggml_free(ctx->kv_ctx); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->buf_w_cpu) - ggml_backend_buffer_free(ctx->buf_w_cpu); + core_gguf::release_weight_buffer(ctx->buf_w_cpu); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/audioseal.cpp b/src/audioseal.cpp index 1c1240985..0c4798264 100644 --- a/src/audioseal.cpp +++ b/src/audioseal.cpp @@ -317,7 +317,7 @@ struct audioseal_ctx { if (ctx_w) ggml_free(ctx_w); if (buf_w) - ggml_backend_buffer_free(buf_w); + core_gguf::release_weight_buffer(buf_w); if (backend && backend != backend_cpu) ggml_backend_free(backend); if (backend_cpu) diff --git a/src/bananamind_tts.cpp b/src/bananamind_tts.cpp index 8c261bd9b..6664f5497 100644 --- a/src/bananamind_tts.cpp +++ b/src/bananamind_tts.cpp @@ -1315,7 +1315,7 @@ void bananamind_tts_free(struct bananamind_tts_context* ctx) { if (!ctx) return; if (ctx->w_buf) - ggml_backend_buffer_free(ctx->w_buf); + core_gguf::release_weight_buffer(ctx->w_buf); if (ctx->w_ctx) ggml_free(ctx->w_ctx); if (ctx->backend) diff --git a/src/bark_tts.cpp b/src/bark_tts.cpp index 82542d463..fc71a1118 100644 --- a/src/bark_tts.cpp +++ b/src/bark_tts.cpp @@ -244,7 +244,7 @@ struct bark_context { if (ctx_w) ggml_free(ctx_w); if (buf_w) - ggml_backend_buffer_free(buf_w); + core_gguf::release_weight_buffer(buf_w); if (backend && backend != backend_cpu) ggml_backend_free(backend); if (backend_cpu) diff --git a/src/beatrice_phone.cpp b/src/beatrice_phone.cpp index 246d1d4c0..7b4f13187 100644 --- a/src/beatrice_phone.cpp +++ b/src/beatrice_phone.cpp @@ -308,7 +308,7 @@ void beatrice_phone_free(beatrice_phone_context* c) { if (!c) return; if (c->buf_w) - ggml_backend_buffer_free(c->buf_w); + core_gguf::release_weight_buffer(c->buf_w); if (c->ctx_w) ggml_free(c->ctx_w); if (c->backend) diff --git a/src/beatrice_pitch.cpp b/src/beatrice_pitch.cpp index 73133ffa9..0c82cbd65 100644 --- a/src/beatrice_pitch.cpp +++ b/src/beatrice_pitch.cpp @@ -309,7 +309,7 @@ void beatrice_pitch_free(beatrice_pitch_context* ctx) { if (!ctx) return; if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend) diff --git a/src/bert_encoder.cpp b/src/bert_encoder.cpp index 491abe7ac..eb2a693b5 100644 --- a/src/bert_encoder.cpp +++ b/src/bert_encoder.cpp @@ -449,7 +449,7 @@ extern "C" void bert_encoder_free(struct bert_encoder_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->w_buf) - ggml_backend_buffer_free(ctx->w_buf); + core_gguf::release_weight_buffer(ctx->w_buf); if (ctx->w_ctx) ggml_free(ctx->w_ctx); if (ctx->backend_cpu) diff --git a/src/btc_chords.cpp b/src/btc_chords.cpp index 05e7bb8ac..2d8fc19e1 100644 --- a/src/btc_chords.cpp +++ b/src/btc_chords.cpp @@ -341,7 +341,7 @@ void btc_chords_free(btc_chords_context* ctx) { if (!ctx) return; if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend) diff --git a/src/canary.cpp b/src/canary.cpp index 2cccce386..660323c17 100644 --- a/src/canary.cpp +++ b/src/canary.cpp @@ -1409,7 +1409,7 @@ extern "C" void canary_free(struct canary_context* ctx) { ctx->model.pw_q8.free(); ctx->model.qkv_fused.free(); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/canary_ctc.cpp b/src/canary_ctc.cpp index c7d2db82f..b34b592bc 100644 --- a/src/canary_ctc.cpp +++ b/src/canary_ctc.cpp @@ -789,7 +789,7 @@ extern "C" void canary_ctc_free(struct canary_ctc_context* ctx) { if (ctx->model.ctx_f32) ggml_free(ctx->model.ctx_f32); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/canary_qwen.cpp b/src/canary_qwen.cpp index a7fbbee90..6ad92b220 100644 --- a/src/canary_qwen.cpp +++ b/src/canary_qwen.cpp @@ -1332,9 +1332,9 @@ extern "C" void canary_qwen_free(struct canary_qwen_context* ctx) { ctx->model.pw_q8.free(); ctx->model.qkv_fused.free(); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.buf_cpu) - ggml_backend_buffer_free(ctx->model.buf_cpu); + core_gguf::release_weight_buffer(ctx->model.buf_cpu); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/chatterbox.cpp b/src/chatterbox.cpp index 4181f91b2..9bd1fe583 100644 --- a/src/chatterbox.cpp +++ b/src/chatterbox.cpp @@ -914,11 +914,11 @@ struct chatterbox_context { if (voice_ctx_w) ggml_free(voice_ctx_w); if (voice_buf_w) - ggml_backend_buffer_free(voice_buf_w); + core_gguf::release_weight_buffer(voice_buf_w); if (ctx_w) ggml_free(ctx_w); if (buf_w) - ggml_backend_buffer_free(buf_w); + core_gguf::release_weight_buffer(buf_w); if (backend && backend != backend_cpu) ggml_backend_free(backend); if (backend_cpu) @@ -4019,7 +4019,7 @@ static int chatterbox_load_voice_gguf(chatterbox_context* ctx, const char* path) ctx->voice_ctx_w = nullptr; } if (ctx->voice_buf_w) { - ggml_backend_buffer_free(ctx->voice_buf_w); + core_gguf::release_weight_buffer(ctx->voice_buf_w); ctx->voice_buf_w = nullptr; } ctx->voice_tensors.clear(); @@ -4242,7 +4242,7 @@ static int chatterbox_install_native_voice(chatterbox_context* ctx, const float ctx->voice_ctx_w = nullptr; } if (ctx->voice_buf_w) { - ggml_backend_buffer_free(ctx->voice_buf_w); + core_gguf::release_weight_buffer(ctx->voice_buf_w); ctx->voice_buf_w = nullptr; } ctx->voice_tensors.clear(); diff --git a/src/chatterbox_s3gen.cpp b/src/chatterbox_s3gen.cpp index 7b5a56730..0a57c3711 100644 --- a/src/chatterbox_s3gen.cpp +++ b/src/chatterbox_s3gen.cpp @@ -414,9 +414,9 @@ struct chatterbox_s3gen_context { if (ctx_w) ggml_free(ctx_w); if (buf_w) - ggml_backend_buffer_free(buf_w); + core_gguf::release_weight_buffer(buf_w); if (buf_cpu_w) - ggml_backend_buffer_free(buf_cpu_w); + core_gguf::release_weight_buffer(buf_cpu_w); if (backend && backend != backend_cpu) ggml_backend_free(backend); if (backend_cpu) diff --git a/src/cohere.cpp b/src/cohere.cpp index 377058a03..883156898 100644 --- a/src/cohere.cpp +++ b/src/cohere.cpp @@ -2103,7 +2103,7 @@ void cohere_free(struct cohere_context* ctx) { if (ctx->kv_buf) ggml_backend_buffer_free(ctx->kv_buf); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->ggml_backend) ggml_backend_free(ctx->ggml_backend); if (ctx->ggml_backend_cpu && ctx->ggml_backend_cpu != ctx->ggml_backend) diff --git a/src/core/gguf_loader.cpp b/src/core/gguf_loader.cpp index c185b22b8..2bf329f0c 100644 --- a/src/core/gguf_loader.cpp +++ b/src/core/gguf_loader.cpp @@ -347,15 +347,21 @@ static const ggml_backend_buffer_i mmap_buffer_iface = { // every weight — the kokoro Metal gibberish-audio regression. // // Instead we hand the inner buffer back as-is and track the mmap region -// in this static side-map. When the buffer is freed elsewhere (model -// shutdown) the inner backend's free callback releases its device-side -// reference, but the host mmap stays mapped — Metal's -// `newBufferWithBytesNoCopy:options:deallocator:nil` doesn't own the -// host pages, so there's no MTLBuffer-side teardown that could munmap. -// We deliberately leak the mmap; on macOS the kernel can still evict -// file-backed pages under pressure (they're not anonymous), and process -// exit reclaims everything. Address-space-wise this costs nothing past -// the model's working set, which we'd be holding anyway. +// in this static side-map. The inner backend's free callback releases its +// device-side reference but cannot touch the host mapping — Metal's +// `newBufferWithBytesNoCopy:options:deallocator:nil` doesn't own the host +// pages, and `buffer_from_host_ptr` has no deallocator parameter through +// which a backend could take ownership of them. Releasing the mapping is +// therefore the loader's job, and `release_weight_buffer()` below is where +// it happens: it frees the backend buffer and then unmaps the region the +// side-map recorded for it. +// +// The mapping is `MAP_PRIVATE | PROT_READ|PROT_WRITE` (see MappedFile's +// `writable` parameter, needed by backends that fold weights in place after +// load), so every page privatizes on first read and the resident pages are +// dirty and anonymous. They can be compressed or swapped, never dropped — +// which is why holding them costs real memory for the life of the process +// rather than page cache the kernel can reclaim. struct gpu_mmap_handle { void* base = nullptr; size_t size = 0; @@ -372,6 +378,64 @@ static gpu_mmap_handle lookup_gpu_mmap(ggml_backend_buffer_t buf) { auto it = g_gpu_mmap.find(buf); return it != g_gpu_mmap.end() ? it->second : gpu_mmap_handle{}; } +// Look the region up and remove the entry in one critical section. Splitting +// this into a lookup followed by an erase would let a second release of the +// same buffer read the entry before the first erased it and unmap twice; it +// would also race a concurrent load whose fresh buffer landed on the same +// address after the free. A default-constructed handle means no entry, which +// is the ordinary case for the CPU mmap path and the legacy alloc+copy path. +static gpu_mmap_handle take_gpu_mmap(ggml_backend_buffer_t buf) { + std::lock_guard lk(g_gpu_mmap_mu); + auto it = g_gpu_mmap.find(buf); + if (it == g_gpu_mmap.end()) + return gpu_mmap_handle{}; + const gpu_mmap_handle h = it->second; + g_gpu_mmap.erase(it); + return h; +} + +static void unmap_region(void* base, size_t size) { + if (!base || size == 0) + return; +#if defined(_WIN32) + (void)size; + UnmapViewOfFile(base); +#else + ::munmap(base, size); +#endif +} + +// Issue #276 overflow chunks. A partition larger than the 1.5 GiB chunk limit +// is allocated as several backend buffers; only the first goes into +// WeightLoad::buf / ::buf_cpu, and the rest were left for the caller to free +// out of WeightLoad::split_bufs. Every one of the eighteen backends that calls +// load_weights_split() moves buf and buf_cpu into its own model struct and +// drops the vector, so those chunks were freed by nobody. An obligation that +// no caller has ever honoured belongs somewhere else. +// +// The overflow chunks are therefore owned here, keyed to the first buffer of +// their own partition, and released with it. split_bufs still lists them so a +// caller can see how a load was partitioned, but it is no longer a set of +// handles the caller must free. +static std::mutex g_split_mu; +static std::map> g_split_extra; + +static void register_split_extra(ggml_backend_buffer_t primary, const std::vector& extra) { + if (!primary || extra.empty()) + return; + std::lock_guard lk(g_split_mu); + auto& slot = g_split_extra[primary]; + slot.insert(slot.end(), extra.begin(), extra.end()); +} +static std::vector take_split_extra(ggml_backend_buffer_t primary) { + std::lock_guard lk(g_split_mu); + auto it = g_split_extra.find(primary); + if (it == g_split_extra.end()) + return {}; + std::vector extra = std::move(it->second); + g_split_extra.erase(it); + return extra; +} // Issue #94 (chatterbox-turbo segfault during init on macOS / Apple // Silicon): the legacy alloc+copy load path takes 30-60 s for the @@ -617,8 +681,7 @@ static bool load_weights_impl(const char* path, ggml_backend_t backend, IncludeT } // Bounds check failed — release the mmap buffer and fall through to // the legacy alloc+copy path below. - ggml_backend_buffer_free(out.buf); - out.buf = nullptr; + release_weight_buffer(out.buf); } // mmap failed or bounds check failed — fall through to the legacy // alloc + copy path. Functionally equivalent, just with more RSS. @@ -640,10 +703,12 @@ static bool load_weights_impl(const char* path, ggml_backend_t backend, IncludeT // pierces the iface abstraction and casts `buffer->context` straight // to its `ggml_metal_buffer_t` — wrapping made Metal read garbage // and emit "tensor 'X' buffer is nil" for every weight (kokoro - // gibberish-audio regression). The mmap region is registered in - // g_gpu_mmap and deliberately leaked when the buffer is freed: Metal - // doesn't own the pages (deallocator=nil), and on macOS file-backed - // pages can still be evicted under pressure. Process exit cleans up. + // gibberish-audio regression). The mmap region is instead recorded in + // g_gpu_mmap against the buffer, and release_weight_buffer() unmaps it + // after freeing that buffer. Metal does not own the pages + // (deallocator=nil) and `buffer_from_host_ptr` offers no way to hand it + // ownership, so the caller must release through that entry point rather + // than through ggml_backend_buffer_free(). if (mmap_loader_enabled() && !ggml_backend_is_cpu(backend)) { ggml_backend_dev_t dev = ggml_backend_get_device(backend); ggml_backend_dev_props props{}; @@ -733,10 +798,12 @@ static bool load_weights_impl(const char* path, ggml_backend_t backend, IncludeT } } if (!bounds_ok) { - out.buf = nullptr; - // inner is registered in g_gpu_mmap and deliberately - // leaked (same as the normal teardown path). - // Fall through to legacy path. + // Release before falling through to the legacy path. + // Abandoning `inner` here would leak both the backend + // buffer and the whole-file mapping registered for it, + // and the failure that reaches this branch is a + // truncated or crafted GGUF — attacker-reachable. + release_weight_buffer(out.buf); } else { for (ggml_tensor* t = ggml_get_first_tensor(out.ctx); t; t = ggml_get_next_tensor(out.ctx, t)) { out.tensors[ggml_get_name(t)] = t; @@ -804,8 +871,7 @@ static bool load_weights_impl(const char* path, ggml_backend_t backend, IncludeT if (!fp) { fprintf(stderr, "%s: cannot open '%s' for fread fallback\n", tag, path); gguf_free(gctx); - ggml_backend_buffer_free(out.buf); - out.buf = nullptr; + release_weight_buffer(out.buf); ggml_free(out.ctx); out.ctx = nullptr; return false; @@ -847,8 +913,7 @@ static bool load_weights_impl(const char* path, ggml_backend_t backend, IncludeT if (!load_ok) { fprintf(stderr, "%s: legacy loader failed — model file may be truncated or corrupt\n", tag); gguf_free(gctx); - ggml_backend_buffer_free(out.buf); - out.buf = nullptr; + release_weight_buffer(out.buf); ggml_free(out.ctx); out.ctx = nullptr; return false; @@ -872,8 +937,7 @@ static bool load_weights_impl(const char* path, ggml_backend_t backend, IncludeT "(off=%zu + nbytes=%zu > file_size=%zu) — file truncated?\n", tag, ggml_get_name(t), data_off + off, nbytes, mf.size); gguf_free(gctx); - ggml_backend_buffer_free(out.buf); - out.buf = nullptr; + release_weight_buffer(out.buf); ggml_free(out.ctx); out.ctx = nullptr; return false; @@ -900,18 +964,33 @@ bool load_weights_filtered(const char* path, ggml_backend_t backend, IncludeTens return load_weights_impl(path, backend, include_tensor, user, model_tag, out); } +void release_weight_buffer(ggml_backend_buffer_t& buf) { + if (!buf) + return; + // Take the entry before the free, not after: between a free and a later + // erase, a concurrent load could receive a new buffer at the same address + // and register it, and the erase would then drop a live mapping's record. + const gpu_mmap_handle h = take_gpu_mmap(buf); + // Overflow chunks of the same partition (issue #276) are released with it. + for (ggml_backend_buffer_t extra : take_split_extra(buf)) + ggml_backend_buffer_free(extra); + // Free the backend buffer first. Metal's shared-storage MTLBuffer is a + // view onto these pages, so unmapping them while the buffer is alive would + // leave the GPU addressing unmapped memory. Freeing first inherits ggml's + // existing caller contract — ggml_metal_buffer_free already vm_deallocates + // the host pages of a buffer it owns, so "no work referencing this buffer + // may still be in flight" is a precondition every caller already meets. + ggml_backend_buffer_free(buf); + buf = nullptr; + unmap_region(h.base, h.size); +} + void free_weights(WeightLoad& wl) { - if (wl.buf) { - ggml_backend_buffer_free(wl.buf); - wl.buf = nullptr; - } - if (wl.buf_cpu) { - ggml_backend_buffer_free(wl.buf_cpu); - wl.buf_cpu = nullptr; - } - // Issue #276: free any overflow chunk buffers from split allocation. - for (auto* b : wl.split_bufs) - ggml_backend_buffer_free(b); + // Issue #276: the overflow chunks in split_bufs are released with the + // primary buffer of their partition, so freeing them again here would be + // a double free. Clearing the vector drops the now-dangling handles. + release_weight_buffer(wl.buf); + release_weight_buffer(wl.buf_cpu); wl.split_bufs.clear(); if (wl.ctx) { ggml_free(wl.ctx); @@ -1001,7 +1080,17 @@ bool load_weights_split(const char* path, ggml_backend_t gpu_backend, ggml_backe // than that need to be split across multiple backend buffers. We chunk // tensors into groups of <= 1.5 GiB each and allocate one buffer per // chunk; the 1.5 GiB limit leaves headroom for alignment padding. - static constexpr size_t max_alloc_chunk = (size_t)1536 * 1024 * 1024; // 1.5 GiB + // + // CRISPASR_GGUF_MAX_ALLOC_CHUNK (bytes) lowers the limit. A driver with a + // tighter cap than AMD's is the field use; the test use is that reaching + // the chunked path otherwise costs a multi-gigabyte allocation, so without + // this the branch that produces overflow buffers has no coverage at all. + size_t max_alloc_chunk = (size_t)1536 * 1024 * 1024; // 1.5 GiB + if (const char* v = std::getenv("CRISPASR_GGUF_MAX_ALLOC_CHUNK")) { + const long long parsed = std::atoll(v); + if (parsed > 0) + max_alloc_chunk = (size_t)parsed; + } auto round_up = [](size_t n, size_t a) { return (n + a - 1) & ~(a - 1); }; auto bind_partition = [&](ggml_backend_t be, const std::vector& tensors, @@ -1051,8 +1140,8 @@ bool load_weights_split(const char* path, ggml_backend_t gpu_backend, ggml_backe ggml_backend_buffer_t buf = ggml_backend_alloc_buffer(be, chunk.aligned_total); if (!buf) { fprintf(stderr, "%s: failed to allocate %zu MiB backend buffer\n", tag, chunk.aligned_total / 1048576); - for (auto* b : out_bufs) - ggml_backend_buffer_free(b); + for (auto& b : out_bufs) + release_weight_buffer(b); out_bufs.clear(); return false; } @@ -1076,25 +1165,28 @@ bool load_weights_split(const char* path, ggml_backend_t gpu_backend, ggml_backe return false; } if (!bind_partition(cpu_backend, cpu_tensors, cpu_bufs)) { - for (auto* b : gpu_bufs) - ggml_backend_buffer_free(b); + for (auto& b : gpu_bufs) + release_weight_buffer(b); gguf_free(gctx); ggml_free(out.ctx); out.ctx = nullptr; return false; } - // First buffer of each partition goes into the canonical fields; - // any overflow chunks go into split_bufs for lifetime management. + // First buffer of each partition goes into the canonical fields; the + // overflow chunks are owned by this loader and released with the primary + // buffer of their own partition. split_bufs lists them for inspection. if (!gpu_bufs.empty()) { out.buf = gpu_bufs[0]; - for (size_t i = 1; i < gpu_bufs.size(); i++) - out.split_bufs.push_back(gpu_bufs[i]); + const std::vector extra(gpu_bufs.begin() + 1, gpu_bufs.end()); + register_split_extra(out.buf, extra); + out.split_bufs.insert(out.split_bufs.end(), extra.begin(), extra.end()); } if (!cpu_bufs.empty()) { out.buf_cpu = cpu_bufs[0]; - for (size_t i = 1; i < cpu_bufs.size(); i++) - out.split_bufs.push_back(cpu_bufs[i]); + const std::vector extra(cpu_bufs.begin() + 1, cpu_bufs.end()); + register_split_extra(out.buf_cpu, extra); + out.split_bufs.insert(out.split_bufs.end(), extra.begin(), extra.end()); } // Copy tensor data from the file. Use mmap when available for zero- diff --git a/src/core/gguf_loader.h b/src/core/gguf_loader.h index 600f536dc..1b4fdb901 100755 --- a/src/core/gguf_loader.h +++ b/src/core/gguf_loader.h @@ -118,9 +118,13 @@ struct WeightLoad { // routed off-GPU. Non-null only when load_weights_split() was used. ggml_backend_buffer_t buf_cpu = nullptr; // Issue #276: extra buffers from chunked allocation in load_weights_split(). - // AMD Vulkan (proprietary driver) caps per-allocation at 2 GiB; models - // larger than that are split across multiple backend buffers. The first - // GPU/CPU buffer is in buf/buf_cpu; any overflow chunks live here. + // AMD Vulkan (proprietary driver) caps per-allocation at 2 GiB, so a + // partition is allocated in chunks of at most 1.5 GiB. The first GPU/CPU + // buffer is in buf/buf_cpu; any overflow chunks are listed here. + // + // READ-ONLY. These handles are owned by the loader and are released with + // the primary buffer of their own partition, so freeing one here is a + // double free. The list is for inspecting how a load was partitioned. std::vector split_bufs; tensor_map tensors; }; @@ -151,8 +155,10 @@ bool load_weights_filtered(const char* path, ggml_backend_t backend, IncludeTens // follow weight residency, giving llama.cpp's `--n-gpu-layers` behaviour. // // Caller takes ownership of `out.ctx`, `out.buf` (gpu partition), and -// `out.buf_cpu` (cpu partition). All three must be freed by the caller -// or via free_weights() / free_weights_split() at shutdown. +// `out.buf_cpu` (cpu partition), and must free all three — the buffers with +// release_weight_buffer(), the context with ggml_free() — or hand the whole +// WeightLoad to free_weights(). Overflow chunks are NOT the caller's to free: +// see the note on WeightLoad::split_bufs. // // Falls back to the legacy alloc+copy path internally — the mmap // optimisations in load_weights() require contiguous tensor regions @@ -195,8 +201,32 @@ bool is_gpu_tensor_with_prefix(const char* tensor_name, void* user); // LayerSplitConfig{ "blk.", *N }. bool is_gpu_tensor_blk(const char* tensor_name, void* user); +// Free a backend buffer that came from one of this loader's weight-loading +// entry points, releasing the host mmap behind it when there is one. +// +// USE THIS INSTEAD OF ggml_backend_buffer_free() FOR ANY BUFFER OBTAINED FROM +// load_weights() / load_weights_filtered() / load_weights_split(), including +// after the buffer has been moved into a model struct. On a non-CPU backend +// advertising `buffer_from_host_ptr` (Apple-Silicon Metal), load_weights hands +// the device a host mmap it does not own — `buffer_from_host_ptr` has no +// deallocator parameter, so freeing the backend buffer alone leaves the whole +// weight file mapped, resident and dirty for the life of the process. +// +// Semantics: +// * A null handle is a no-op, and the caller's handle is nulled on return, +// so a second call cannot double-free or double-unmap. +// * A buffer with no recorded mapping is the ordinary case — the CPU mmap +// path unmaps through its own free callback and the legacy alloc+copy +// path maps nothing — and is released like any other backend buffer. +// * The backend buffer is freed before the region is unmapped, so a +// device-side view of the pages never outlives them. +// * Any overflow chunks of the same partition (issue #276) are released +// with it, so a split load needs no separate teardown. +void release_weight_buffer(ggml_backend_buffer_t& buf); + // Free a WeightLoad's resources. Call when the model is being destroyed -// and the buffer/context are not held elsewhere. +// and the buffer/context are not held elsewhere. Releases every buffer +// through release_weight_buffer(). void free_weights(WeightLoad& wl); // PLAN #60g: hint the kernel that the mmap'd weight region is now being diff --git a/src/core/snac.cpp b/src/core/snac.cpp index 8d5fa0532..63b477a3e 100644 --- a/src/core/snac.cpp +++ b/src/core/snac.cpp @@ -158,7 +158,7 @@ struct snac_decoder_ctx { ggml_free(ctx_w); } if (buf_w) { - ggml_backend_buffer_free(buf_w); + core_gguf::release_weight_buffer(buf_w); } if (backend && backend != backend_cpu) { ggml_backend_free(backend); diff --git a/src/cosyvoice3_tts.cpp b/src/cosyvoice3_tts.cpp index fbf6ed925..07a6c9ed9 100644 --- a/src/cosyvoice3_tts.cpp +++ b/src/cosyvoice3_tts.cpp @@ -1249,26 +1249,26 @@ extern "C" void cosyvoice3_tts_free(struct cosyvoice3_tts_context* ctx) { if (ctx->cpu_gallocr) ggml_gallocr_free(ctx->cpu_gallocr); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->buf_w_cpu) - ggml_backend_buffer_free(ctx->buf_w_cpu); + core_gguf::release_weight_buffer(ctx->buf_w_cpu); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->flow.buf_w) - ggml_backend_buffer_free(ctx->flow.buf_w); + core_gguf::release_weight_buffer(ctx->flow.buf_w); if (ctx->flow.ctx_w) ggml_free(ctx->flow.ctx_w); ctx->hift.hift_fc.free(); // FASTCONV baked kernels (before the backend is freed) if (ctx->hift.buf_w) - ggml_backend_buffer_free(ctx->hift.buf_w); + core_gguf::release_weight_buffer(ctx->hift.buf_w); if (ctx->hift.ctx_w) ggml_free(ctx->hift.ctx_w); if (ctx->s3tok.buf_w) - ggml_backend_buffer_free(ctx->s3tok.buf_w); + core_gguf::release_weight_buffer(ctx->s3tok.buf_w); if (ctx->s3tok.ctx_w) ggml_free(ctx->s3tok.ctx_w); if (ctx->campplus.buf_w) - ggml_backend_buffer_free(ctx->campplus.buf_w); + core_gguf::release_weight_buffer(ctx->campplus.buf_w); if (ctx->campplus.ctx_w) ggml_free(ctx->campplus.ctx_w); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/csm_tts.cpp b/src/csm_tts.cpp index d48a1698b..2f988f206 100644 --- a/src/csm_tts.cpp +++ b/src/csm_tts.cpp @@ -296,7 +296,7 @@ struct csm_tts_context { if (model.ctx_perm) ggml_free(model.ctx_perm); if (model.buf_w) - ggml_backend_buffer_free(model.buf_w); + core_gguf::release_weight_buffer(model.buf_w); if (model.ctx_w) ggml_free(model.ctx_w); if (backend && backend != backend_cpu) diff --git a/src/dia_tts.cpp b/src/dia_tts.cpp index ce524891c..61cbefdd4 100644 --- a/src/dia_tts.cpp +++ b/src/dia_tts.cpp @@ -1020,7 +1020,7 @@ int dia_tts_set_codec_path(struct dia_tts_context* ctx, const char* path) { if (!dia_load_dac_weights(m, wl.tensors, verbosity)) { fprintf(stderr, "dia_tts: failed to map DAC codec tensors\n"); if (wl.buf) - ggml_backend_buffer_free(wl.buf); + core_gguf::release_weight_buffer(wl.buf); if (wl.ctx) ggml_free(wl.ctx); return -1; @@ -2170,7 +2170,7 @@ void dia_tts_free(struct dia_tts_context* ctx) { ggml_backend_buffer_free(ctx->buf_output); } if (ctx->model.buf_w) { - ggml_backend_buffer_free(ctx->model.buf_w); + core_gguf::release_weight_buffer(ctx->model.buf_w); } if (ctx->model.ctx_w) { ggml_free(ctx->model.ctx_w); @@ -2182,7 +2182,7 @@ void dia_tts_free(struct dia_tts_context* ctx) { ggml_free(ctx->model.ctx_perm); } if (ctx->model.buf_dac) { - ggml_backend_buffer_free(ctx->model.buf_dac); + core_gguf::release_weight_buffer(ctx->model.buf_dac); } if (ctx->model.ctx_dac) { ggml_free(ctx->model.ctx_dac); diff --git a/src/dots_tts.cpp b/src/dots_tts.cpp index 1189274f3..cd33bd60a 100644 --- a/src/dots_tts.cpp +++ b/src/dots_tts.cpp @@ -2850,12 +2850,12 @@ void dots_tts_free(struct dots_tts_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->voc.buf_w) - ggml_backend_buffer_free(ctx->voc.buf_w); + core_gguf::release_weight_buffer(ctx->voc.buf_w); if (ctx->voc.ctx_w) ggml_free(ctx->voc.ctx_w); diff --git a/src/ecapa_lid.cpp b/src/ecapa_lid.cpp index a1eab1e38..f997a0d7b 100644 --- a/src/ecapa_lid.cpp +++ b/src/ecapa_lid.cpp @@ -350,7 +350,7 @@ extern "C" void ecapa_lid_free(struct ecapa_lid_context* ctx) { if (ctx->weight_ctx) ggml_free(ctx->weight_ctx); if (ctx->buf) - ggml_backend_buffer_free(ctx->buf); + core_gguf::release_weight_buffer(ctx->buf); if (ctx->backend) ggml_backend_free(ctx->backend); delete ctx; diff --git a/src/f5_tts.cpp b/src/f5_tts.cpp index 79ba0b2d1..dfb97a016 100644 --- a/src/f5_tts.cpp +++ b/src/f5_tts.cpp @@ -2221,7 +2221,7 @@ void f5_tts_free(struct f5_tts_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->w_buf) - ggml_backend_buffer_free(ctx->w_buf); + core_gguf::release_weight_buffer(ctx->w_buf); if (ctx->w_ctx) ggml_free(ctx->w_ctx); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/fastpitch_tts.cpp b/src/fastpitch_tts.cpp index c9461cf34..b74c4f036 100644 --- a/src/fastpitch_tts.cpp +++ b/src/fastpitch_tts.cpp @@ -1202,7 +1202,7 @@ void fastpitch_tts_free(struct fastpitch_tts_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/firered_asr.cpp b/src/firered_asr.cpp index 1cedb4dea..366842848 100644 --- a/src/firered_asr.cpp +++ b/src/firered_asr.cpp @@ -640,9 +640,9 @@ extern "C" void firered_asr_free(struct firered_asr_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.buf_cpu) - ggml_backend_buffer_free(ctx->model.buf_cpu); + core_gguf::release_weight_buffer(ctx->model.buf_cpu); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/fireredpunc.cpp b/src/fireredpunc.cpp index 8ae814390..9fe6b909a 100644 --- a/src/fireredpunc.cpp +++ b/src/fireredpunc.cpp @@ -905,7 +905,7 @@ void fireredpunc_free(fireredpunc_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->buf) - ggml_backend_buffer_free(ctx->buf); + core_gguf::release_weight_buffer(ctx->buf); if (ctx->w_ctx) ggml_free(ctx->w_ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/funasr.cpp b/src/funasr.cpp index 9e302cb35..38767ea5a 100644 --- a/src/funasr.cpp +++ b/src/funasr.cpp @@ -2228,9 +2228,9 @@ extern "C" void funasr_free(funasr_context* ctx) { if (ctx->fused_ctx) ggml_free(ctx->fused_ctx); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.buf_cpu) - ggml_backend_buffer_free(ctx->model.buf_cpu); + core_gguf::release_weight_buffer(ctx->model.buf_cpu); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend) diff --git a/src/gemma4_e2b.cpp b/src/gemma4_e2b.cpp index 3ff1b83ef..f631fc01f 100644 --- a/src/gemma4_e2b.cpp +++ b/src/gemma4_e2b.cpp @@ -2716,9 +2716,9 @@ extern "C" void gemma4_e2b_free(struct gemma4_e2b_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->model.buf_w) - ggml_backend_buffer_free(ctx->model.buf_w); + core_gguf::release_weight_buffer(ctx->model.buf_w); if (ctx->model.buf_w_cpu) - ggml_backend_buffer_free(ctx->model.buf_w_cpu); + core_gguf::release_weight_buffer(ctx->model.buf_w_cpu); if (ctx->model.ctx_w) ggml_free(ctx->model.ctx_w); if (ctx->backend_cpu) diff --git a/src/gigaam.cpp b/src/gigaam.cpp index 9da5d80af..52e55450c 100644 --- a/src/gigaam.cpp +++ b/src/gigaam.cpp @@ -1114,7 +1114,7 @@ extern "C" void gigaam_free(struct gigaam_context* ctx) { ggml_backend_sched_free(ctx->sched); ctx->model.pw_q8.free(); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/glm_asr.cpp b/src/glm_asr.cpp index 8c18c4605..d9183feb0 100644 --- a/src/glm_asr.cpp +++ b/src/glm_asr.cpp @@ -503,9 +503,9 @@ extern "C" void glm_asr_free(struct glm_asr_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.buf_cpu) - ggml_backend_buffer_free(ctx->model.buf_cpu); + core_gguf::release_weight_buffer(ctx->model.buf_cpu); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/granite_nle.cpp b/src/granite_nle.cpp index a29cfc368..08b32f530 100644 --- a/src/granite_nle.cpp +++ b/src/granite_nle.cpp @@ -723,7 +723,7 @@ extern "C" void granite_nle_free(struct granite_nle_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/granite_speech.cpp b/src/granite_speech.cpp index 1a707da95..b7421e437 100644 --- a/src/granite_speech.cpp +++ b/src/granite_speech.cpp @@ -876,9 +876,9 @@ extern "C" void granite_speech_free(struct granite_speech_context* ctx) { if (ctx->kv_ctx) ggml_free(ctx->kv_ctx); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.buf_cpu) - ggml_backend_buffer_free(ctx->model.buf_cpu); + core_gguf::release_weight_buffer(ctx->model.buf_cpu); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/higgs_stt.cpp b/src/higgs_stt.cpp index 37932a649..345764f4b 100644 --- a/src/higgs_stt.cpp +++ b/src/higgs_stt.cpp @@ -1444,9 +1444,9 @@ extern "C" void higgs_stt_free(higgs_stt_context* ctx) { if (ctx->kv_ctx) ggml_free(ctx->kv_ctx); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.buf_cpu) - ggml_backend_buffer_free(ctx->model.buf_cpu); + core_gguf::release_weight_buffer(ctx->model.buf_cpu); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu) diff --git a/src/htdemucs.cpp b/src/htdemucs.cpp index 1489b8b57..d1f0819b9 100644 --- a/src/htdemucs.cpp +++ b/src/htdemucs.cpp @@ -832,7 +832,7 @@ void htdemucs_free(htdemucs_context* ctx) { if (!ctx) return; if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend) diff --git a/src/indextts.cpp b/src/indextts.cpp index 741c0cdc0..eb5de3152 100644 --- a/src/indextts.cpp +++ b/src/indextts.cpp @@ -725,7 +725,7 @@ struct indextts_context { ggml_free(ctx_w); } if (buf_w) { - ggml_backend_buffer_free(buf_w); + core_gguf::release_weight_buffer(buf_w); } if (backend && backend != backend_cpu) { ggml_backend_free(backend); diff --git a/src/indextts_voc.cpp b/src/indextts_voc.cpp index 574bfdab0..9dfa1e5f0 100644 --- a/src/indextts_voc.cpp +++ b/src/indextts_voc.cpp @@ -191,7 +191,7 @@ struct indextts_voc_context { ggml_free(ctx_w); } if (buf_w) { - ggml_backend_buffer_free(buf_w); + core_gguf::release_weight_buffer(buf_w); } if (backend && backend != backend_cpu) { ggml_backend_free(backend); diff --git a/src/irodori_tts.cpp b/src/irodori_tts.cpp index 134de8f17..ed290ac15 100644 --- a/src/irodori_tts.cpp +++ b/src/irodori_tts.cpp @@ -1479,11 +1479,11 @@ void irodori_tts_free(struct irodori_tts_context* ctx) { return; ctx->dac_fc.free(); // FASTCONV baked-kernel buffer (on codec_backend) if (ctx->codec_buf) - ggml_backend_buffer_free(ctx->codec_buf); + core_gguf::release_weight_buffer(ctx->codec_buf); if (ctx->codec_ctx) ggml_free(ctx->codec_ctx); if (ctx->buf_weights) - ggml_backend_buffer_free(ctx->buf_weights); + core_gguf::release_weight_buffer(ctx->buf_weights); if (ctx->w_ctx) ggml_free(ctx->w_ctx); if (ctx->dit_galloc) // §243 persistent DiT graph diff --git a/src/kokoro.cpp b/src/kokoro.cpp index 742165126..4e97ed0ae 100644 --- a/src/kokoro.cpp +++ b/src/kokoro.cpp @@ -2844,7 +2844,7 @@ extern "C" int kokoro_load_voice_pack(struct kokoro_context* ctx, const char* pa // Replace any previously-loaded pack. if (ctx->vp.vp_buf_w) - ggml_backend_buffer_free(ctx->vp.vp_buf_w); + core_gguf::release_weight_buffer(ctx->vp.vp_buf_w); if (ctx->vp.vp_ctx_w) ggml_free(ctx->vp.vp_ctx_w); ctx->vp = std::move(vp); @@ -2863,7 +2863,7 @@ extern "C" int kokoro_load_voice_pack(struct kokoro_context* ctx, const char* pa auto it = wl.tensors.find("voice.pack"); if (it == wl.tensors.end() || !it->second) { fprintf(stderr, "kokoro: voice pack '%s' missing 'voice.pack' tensor\n", path); - ggml_backend_buffer_free(wl.buf); + core_gguf::release_weight_buffer(wl.buf); ggml_free(wl.ctx); return -1; } @@ -3621,7 +3621,7 @@ extern "C" void kokoro_free(struct kokoro_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->vp.vp_buf_w) - ggml_backend_buffer_free(ctx->vp.vp_buf_w); + core_gguf::release_weight_buffer(ctx->vp.vp_buf_w); if (ctx->vp.vp_ctx_w) ggml_free(ctx->vp.vp_ctx_w); if (ctx->buf_perm) @@ -3629,7 +3629,7 @@ extern "C" void kokoro_free(struct kokoro_context* ctx) { if (ctx->ctx_perm) ggml_free(ctx->ctx_perm); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/kugelaudio.cpp b/src/kugelaudio.cpp index f5e4b154a..c1283078b 100644 --- a/src/kugelaudio.cpp +++ b/src/kugelaudio.cpp @@ -535,9 +535,9 @@ extern "C" void kugelaudio_free(struct kugelaudio_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->buf) - ggml_backend_buffer_free(ctx->buf); + core_gguf::release_weight_buffer(ctx->buf); if (ctx->buf_cpu) - ggml_backend_buffer_free(ctx->buf_cpu); + core_gguf::release_weight_buffer(ctx->buf_cpu); if (ctx->weight_ctx) ggml_free(ctx->weight_ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/kyutai_stt.cpp b/src/kyutai_stt.cpp index 399ee3128..bed197756 100644 --- a/src/kyutai_stt.cpp +++ b/src/kyutai_stt.cpp @@ -576,7 +576,7 @@ extern "C" void kyutai_stt_free(struct kyutai_stt_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/lfm2_audio.cpp b/src/lfm2_audio.cpp index e9715f74f..839a5bf05 100644 --- a/src/lfm2_audio.cpp +++ b/src/lfm2_audio.cpp @@ -664,7 +664,7 @@ void lfm2_audio_free(lfm2_audio_context* ctx) { if (!ctx) return; if (ctx->detok.buf) - ggml_backend_buffer_free(ctx->detok.buf); + core_gguf::release_weight_buffer(ctx->detok.buf); if (ctx->detok.ctx) ggml_free(ctx->detok.ctx); if (ctx->sched) @@ -676,7 +676,7 @@ void lfm2_audio_free(lfm2_audio_context* ctx) { ctx->model.pw_q8.free(); ctx->model.qkv_fused.free(); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend) diff --git a/src/lid_cld3.cpp b/src/lid_cld3.cpp index 098e92611..019cab175 100755 --- a/src/lid_cld3.cpp +++ b/src/lid_cld3.cpp @@ -804,7 +804,7 @@ extern "C" void lid_cld3_free(lid_cld3_context* ctx) { if (!ctx) return; if (ctx->buf) - ggml_backend_buffer_free(ctx->buf); + core_gguf::release_weight_buffer(ctx->buf); if (ctx->gctx) ggml_free(ctx->gctx); delete ctx; diff --git a/src/m2m100.cpp b/src/m2m100.cpp index 8be9d2fe2..b48fd8270 100755 --- a/src/m2m100.cpp +++ b/src/m2m100.cpp @@ -1038,7 +1038,7 @@ extern "C" void m2m100_free(struct m2m100_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/mel_band_roformer.cpp b/src/mel_band_roformer.cpp index 7d29ee985..90bf4a61c 100644 --- a/src/mel_band_roformer.cpp +++ b/src/mel_band_roformer.cpp @@ -321,7 +321,7 @@ void mel_band_roformer_free(mel_band_roformer_context* ctx) { if (!ctx) return; if (ctx->weights.buf) - ggml_backend_buffer_free(ctx->weights.buf); + core_gguf::release_weight_buffer(ctx->weights.buf); if (ctx->weights.ctx) ggml_free(ctx->weights.ctx); if (ctx->backend) @@ -1275,7 +1275,7 @@ int mel_band_roformer_diff(const char* model_gguf, const char* ref_gguf, const c } if (rw.buf) - ggml_backend_buffer_free(rw.buf); + core_gguf::release_weight_buffer(rw.buf); if (rw.ctx) ggml_free(rw.ctx); mel_band_roformer_free(ctx); diff --git a/src/melotts.cpp b/src/melotts.cpp index 319ea55ea..1f7013cad 100644 --- a/src/melotts.cpp +++ b/src/melotts.cpp @@ -2805,7 +2805,7 @@ void melotts_free(struct melotts_context* ctx) { if (ctx->ctx_perm) ggml_free(ctx->ctx_perm); if (ctx->w_buf) - ggml_backend_buffer_free(ctx->w_buf); + core_gguf::release_weight_buffer(ctx->w_buf); if (ctx->w_ctx) ggml_free(ctx->w_ctx); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/mimo_asr.cpp b/src/mimo_asr.cpp index cff7c0e4e..e417b2880 100644 --- a/src/mimo_asr.cpp +++ b/src/mimo_asr.cpp @@ -1929,9 +1929,9 @@ extern "C" void mimo_asr_free(struct mimo_asr_context* ctx) { if (ctx->kv_ctx) ggml_free(ctx->kv_ctx); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->buf_w_cpu) - ggml_backend_buffer_free(ctx->buf_w_cpu); + core_gguf::release_weight_buffer(ctx->buf_w_cpu); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/mimo_tokenizer.cpp b/src/mimo_tokenizer.cpp index 7f6485f9b..b2be41245 100644 --- a/src/mimo_tokenizer.cpp +++ b/src/mimo_tokenizer.cpp @@ -552,7 +552,7 @@ extern "C" void mimo_tokenizer_free(struct mimo_tokenizer_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/mini_omni2.cpp b/src/mini_omni2.cpp index 561f572a4..a988a4164 100644 --- a/src/mini_omni2.cpp +++ b/src/mini_omni2.cpp @@ -420,9 +420,9 @@ extern "C" void mini_omni2_free(struct mini_omni2_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.buf_cpu) - ggml_backend_buffer_free(ctx->model.buf_cpu); + core_gguf::release_weight_buffer(ctx->model.buf_cpu); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/miocodec.cpp b/src/miocodec.cpp index 76a0827b6..b36b72296 100644 --- a/src/miocodec.cpp +++ b/src/miocodec.cpp @@ -477,7 +477,7 @@ void miocodec_free(struct miocodec_context* ctx) { if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->backend) ggml_backend_free(ctx->backend); delete ctx; diff --git a/src/moonshine-impl.h b/src/moonshine-impl.h index 6820cc8b5..36ce97c73 100755 --- a/src/moonshine-impl.h +++ b/src/moonshine-impl.h @@ -1,5 +1,7 @@ #pragma once +#include "core/gguf_loader.h" + #include "ggml.h" #include "ggml-backend.h" @@ -129,8 +131,9 @@ struct moonshine_model { moonshine_model() = default; ~moonshine_model() { - ggml_backend_buffer_free(buf_w); - ggml_backend_buffer_free(buf_w_cpu); + // Weight buffers from core_gguf::load_weights / load_weights_split. + core_gguf::release_weight_buffer(buf_w); + core_gguf::release_weight_buffer(buf_w_cpu); ggml_free(ctx_w); } diff --git a/src/moonshine_streaming.cpp b/src/moonshine_streaming.cpp index b85820e7a..73e293ef2 100644 --- a/src/moonshine_streaming.cpp +++ b/src/moonshine_streaming.cpp @@ -1169,9 +1169,9 @@ extern "C" void moonshine_streaming_free(struct moonshine_streaming_context* ctx if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->model.buf_w) - ggml_backend_buffer_free(ctx->model.buf_w); + core_gguf::release_weight_buffer(ctx->model.buf_w); if (ctx->model.buf_w_cpu) - ggml_backend_buffer_free(ctx->model.buf_w_cpu); + core_gguf::release_weight_buffer(ctx->model.buf_w_cpu); if (ctx->model.ctx_w) ggml_free(ctx->model.ctx_w); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/moss_audio.cpp b/src/moss_audio.cpp index 331d6728a..05ded7e6f 100644 --- a/src/moss_audio.cpp +++ b/src/moss_audio.cpp @@ -2202,7 +2202,7 @@ extern "C" void moss_audio_free(struct moss_audio_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/moss_transcribe.cpp b/src/moss_transcribe.cpp index 5a19dfe49..b8443bfce 100644 --- a/src/moss_transcribe.cpp +++ b/src/moss_transcribe.cpp @@ -1613,7 +1613,7 @@ extern "C" void moss_transcribe_free(struct moss_transcribe_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/moss_transcribe_diarize.cpp b/src/moss_transcribe_diarize.cpp index 912c3e007..ec9eac18e 100644 --- a/src/moss_transcribe_diarize.cpp +++ b/src/moss_transcribe_diarize.cpp @@ -1642,7 +1642,7 @@ extern "C" void moss_diarize_free(struct moss_diarize_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/moss_tts.cpp b/src/moss_tts.cpp index ec7fe8335..9c4f27d24 100644 --- a/src/moss_tts.cpp +++ b/src/moss_tts.cpp @@ -1516,7 +1516,7 @@ extern "C" void moss_tts_free(moss_tts_context* ctx) { if (ctx->kv_ctx) ggml_free(ctx->kv_ctx); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu) diff --git a/src/moss_tts_codec.cpp b/src/moss_tts_codec.cpp index 714a17ebf..59a1e2b14 100644 --- a/src/moss_tts_codec.cpp +++ b/src/moss_tts_codec.cpp @@ -461,7 +461,7 @@ void free(Codec* c) { if (c->w_ctx) ggml_free(c->w_ctx); if (c->buf) - ggml_backend_buffer_free(c->buf); + core_gguf::release_weight_buffer(c->buf); if (c->ctx) ggml_free(c->ctx); delete c; diff --git a/src/moss_tts_local.cpp b/src/moss_tts_local.cpp index e1cbb4c44..ffc13b63c 100644 --- a/src/moss_tts_local.cpp +++ b/src/moss_tts_local.cpp @@ -1437,7 +1437,7 @@ extern "C" void moss_tts_local_free(moss_tts_local_context* ctx) { if (ctx->kv_ctx) ggml_free(ctx->kv_ctx); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/moss_tts_local_codec.cpp b/src/moss_tts_local_codec.cpp index 546aa8f0d..228596c48 100644 --- a/src/moss_tts_local_codec.cpp +++ b/src/moss_tts_local_codec.cpp @@ -560,7 +560,7 @@ void free(Codec* c) { if (c->w_ctx) ggml_free(c->w_ctx); if (c->buf) - ggml_backend_buffer_free(c->buf); + core_gguf::release_weight_buffer(c->buf); if (c->ctx) ggml_free(c->ctx); delete c; diff --git a/src/nemotron.cpp b/src/nemotron.cpp index 6e1b0bcb7..e9390481f 100644 --- a/src/nemotron.cpp +++ b/src/nemotron.cpp @@ -2428,7 +2428,7 @@ extern "C" void nemotron_free(struct nemotron_context* ctx) { ggml_backend_sched_free(ctx->sched); ctx->model.pw_q8.free(); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/omniasr.cpp b/src/omniasr.cpp index 00613abdf..d80e0e0de 100644 --- a/src/omniasr.cpp +++ b/src/omniasr.cpp @@ -573,10 +573,11 @@ extern "C" void omniasr_free(struct omniasr_context* ctx) { ggml_backend_sched_free(ctx->sched); if (ctx->weight_ctx) ggml_free(ctx->weight_ctx); - if (ctx->buf) - ggml_backend_buffer_free(ctx->buf); - if (ctx->buf_cpu) - ggml_backend_buffer_free(ctx->buf_cpu); + // Weight buffers come from core_gguf::load_weights, so on a zero-copy + // backend they carry a host mmap the backend does not own. Release them + // through the loader rather than ggml_backend_buffer_free(). + core_gguf::release_weight_buffer(ctx->buf); + core_gguf::release_weight_buffer(ctx->buf_cpu); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) ggml_backend_free(ctx->backend_cpu); if (ctx->backend) diff --git a/src/openvoice2.cpp b/src/openvoice2.cpp index 7c923c7cb..e0e0f8baa 100644 --- a/src/openvoice2.cpp +++ b/src/openvoice2.cpp @@ -1419,7 +1419,7 @@ extern "C" void openvoice2_free(struct openvoice2_context* ctx) { if (ctx->ctx_perm) ggml_free(ctx->ctx_perm); if (ctx->w_buf) - ggml_backend_buffer_free(ctx->w_buf); + core_gguf::release_weight_buffer(ctx->w_buf); if (ctx->w_ctx) ggml_free(ctx->w_ctx); if (ctx->backend_cpu) diff --git a/src/orpheus.cpp b/src/orpheus.cpp index 170cdf33d..2664857eb 100644 --- a/src/orpheus.cpp +++ b/src/orpheus.cpp @@ -210,10 +210,10 @@ struct orpheus_context { ggml_free(ctx_w); } if (buf_w) { - ggml_backend_buffer_free(buf_w); + core_gguf::release_weight_buffer(buf_w); } if (buf_w_cpu) { - ggml_backend_buffer_free(buf_w_cpu); + core_gguf::release_weight_buffer(buf_w_cpu); } if (backend && backend != backend_cpu) { ggml_backend_free(backend); diff --git a/src/outetts.cpp b/src/outetts.cpp index a4c708242..5e28a3864 100644 --- a/src/outetts.cpp +++ b/src/outetts.cpp @@ -209,10 +209,10 @@ struct outetts_context { ggml_free(ctx_w); } if (buf_w) { - ggml_backend_buffer_free(buf_w); + core_gguf::release_weight_buffer(buf_w); } if (buf_w_cpu) { - ggml_backend_buffer_free(buf_w_cpu); + core_gguf::release_weight_buffer(buf_w_cpu); } if (backend && backend != backend_cpu) { ggml_backend_free(backend); diff --git a/src/outetts_wavtok.cpp b/src/outetts_wavtok.cpp index 3b88a4fe2..5ea2f84dd 100755 --- a/src/outetts_wavtok.cpp +++ b/src/outetts_wavtok.cpp @@ -158,7 +158,7 @@ struct wavtok_decoder_ctx { if (ctx_w) ggml_free(ctx_w); if (buf_w) - ggml_backend_buffer_free(buf_w); + core_gguf::release_weight_buffer(buf_w); if (backend && backend != backend_cpu) ggml_backend_free(backend); if (backend_cpu) diff --git a/src/parakeet.cpp b/src/parakeet.cpp index 44b11b738..5c38aafe1 100644 --- a/src/parakeet.cpp +++ b/src/parakeet.cpp @@ -2920,7 +2920,7 @@ extern "C" void parakeet_free(struct parakeet_context* ctx) { if (ctx->model.ctx_f32) ggml_free(ctx->model.ctx_f32); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/parler_tts.cpp b/src/parler_tts.cpp index 363206822..dc861c713 100644 --- a/src/parler_tts.cpp +++ b/src/parler_tts.cpp @@ -2185,7 +2185,7 @@ void parler_tts_free(struct parler_tts_context* ctx) { if (ctx->ctx_perm) ggml_free(ctx->ctx_perm); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/pcs.cpp b/src/pcs.cpp index 8647a10be..84bd5ce78 100644 --- a/src/pcs.cpp +++ b/src/pcs.cpp @@ -1021,7 +1021,7 @@ void pcs_free(pcs_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->buf) - ggml_backend_buffer_free(ctx->buf); + core_gguf::release_weight_buffer(ctx->buf); if (ctx->w_ctx) ggml_free(ctx->w_ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/piano_transcription.cpp b/src/piano_transcription.cpp index 95c8b6e8b..59413595f 100644 --- a/src/piano_transcription.cpp +++ b/src/piano_transcription.cpp @@ -826,7 +826,7 @@ void piano_transcription_free(struct piano_transcription_ctx* ctx) { if (!ctx) return; if (ctx->w_buf) - ggml_backend_buffer_free(ctx->w_buf); + core_gguf::release_weight_buffer(ctx->w_buf); if (ctx->w_ctx) ggml_free(ctx->w_ctx); if (ctx->backend) diff --git a/src/piper_tts.cpp b/src/piper_tts.cpp index 426e0f068..bfd15b711 100644 --- a/src/piper_tts.cpp +++ b/src/piper_tts.cpp @@ -2357,7 +2357,7 @@ void piper_tts_free(struct piper_tts_context* ctx) { if (ctx->ctx_perm) ggml_free(ctx->ctx_perm); if (ctx->w_buf) - ggml_backend_buffer_free(ctx->w_buf); + core_gguf::release_weight_buffer(ctx->w_buf); if (ctx->w_ctx) ggml_free(ctx->w_ctx); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/pocket_tts.cpp b/src/pocket_tts.cpp index 89e827945..b5c989c20 100644 --- a/src/pocket_tts.cpp +++ b/src/pocket_tts.cpp @@ -3094,7 +3094,7 @@ void pocket_tts_free(struct pocket_tts_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/pyannote_seg.cpp b/src/pyannote_seg.cpp index a8abbc3d3..bcd985613 100644 --- a/src/pyannote_seg.cpp +++ b/src/pyannote_seg.cpp @@ -770,7 +770,7 @@ extern "C" void pyannote_seg_free(struct pyannote_seg_context* ctx) { if (!ctx) return; if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->model.backend) diff --git a/src/qwen3_asr.cpp b/src/qwen3_asr.cpp index e79a9904d..2dc121ff8 100644 --- a/src/qwen3_asr.cpp +++ b/src/qwen3_asr.cpp @@ -1609,9 +1609,9 @@ extern "C" void qwen3_asr_free(qwen3_asr_context* ctx) { if (ctx->kv_ctx) ggml_free(ctx->kv_ctx); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.buf_cpu) - ggml_backend_buffer_free(ctx->model.buf_cpu); + core_gguf::release_weight_buffer(ctx->model.buf_cpu); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu) diff --git a/src/qwen3_tts.cpp b/src/qwen3_tts.cpp index de3228915..f005d0c20 100644 --- a/src/qwen3_tts.cpp +++ b/src/qwen3_tts.cpp @@ -6516,7 +6516,7 @@ extern "C" int qwen3_tts_load_voice_pack(struct qwen3_tts_context* ctx, const ch } if (ctx->vp_buf_w) { - ggml_backend_buffer_free(ctx->vp_buf_w); + core_gguf::release_weight_buffer(ctx->vp_buf_w); } if (ctx->vp_ctx_w) { ggml_free(ctx->vp_ctx_w); @@ -7886,7 +7886,7 @@ extern "C" void qwen3_tts_free(struct qwen3_tts_context* ctx) { ggml_free(ctx->cp_cpu_ctx); } if (ctx->codec.buf_w) { - ggml_backend_buffer_free(ctx->codec.buf_w); + core_gguf::release_weight_buffer(ctx->codec.buf_w); } if (ctx->codec.ctx_w) { ggml_free(ctx->codec.ctx_w); @@ -7904,13 +7904,13 @@ extern "C" void qwen3_tts_free(struct qwen3_tts_context* ctx) { ggml_free(ctx->codec.ctx_conv32); } if (ctx->vp_buf_w) { - ggml_backend_buffer_free(ctx->vp_buf_w); + core_gguf::release_weight_buffer(ctx->vp_buf_w); } if (ctx->vp_ctx_w) { ggml_free(ctx->vp_ctx_w); } if (ctx->buf_w) { - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); } if (ctx->ctx_w) { ggml_free(ctx->ctx_w); diff --git a/src/rvc_svc.cpp b/src/rvc_svc.cpp index 1400ad78d..570a6333f 100644 --- a/src/rvc_svc.cpp +++ b/src/rvc_svc.cpp @@ -232,7 +232,7 @@ void rvc_svc_free(rvc_svc_context* ctx) { if (!ctx) return; if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend) diff --git a/src/sensevoice.cpp b/src/sensevoice.cpp index f6082ae21..8cdf25389 100644 --- a/src/sensevoice.cpp +++ b/src/sensevoice.cpp @@ -719,9 +719,9 @@ extern "C" void sensevoice_free(sensevoice_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.buf_cpu) - ggml_backend_buffer_free(ctx->model.buf_cpu); + core_gguf::release_weight_buffer(ctx->model.buf_cpu); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend) diff --git a/src/sidon.cpp b/src/sidon.cpp index ba7b5ae48..9797758f5 100644 --- a/src/sidon.cpp +++ b/src/sidon.cpp @@ -792,9 +792,9 @@ void sidon_free(sidon_context* ctx) { ggml_backend_sched_free(ctx->decoder_sched); ctx->decoder_fc.free(); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.buf_cpu) - ggml_backend_buffer_free(ctx->model.buf_cpu); + core_gguf::release_weight_buffer(ctx->model.buf_cpu); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend) diff --git a/src/silero_lid.cpp b/src/silero_lid.cpp index 15eec7f10..fbfc57fae 100644 --- a/src/silero_lid.cpp +++ b/src/silero_lid.cpp @@ -869,9 +869,9 @@ extern "C" void silero_lid_free(struct silero_lid_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.buf_cpu) - ggml_backend_buffer_free(ctx->model.buf_cpu); + core_gguf::release_weight_buffer(ctx->model.buf_cpu); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/t5_translate.cpp b/src/t5_translate.cpp index eca3ed827..94db37995 100644 --- a/src/t5_translate.cpp +++ b/src/t5_translate.cpp @@ -1146,7 +1146,7 @@ extern "C" void t5_translate_free(struct t5_translate_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/tada_codec.cpp b/src/tada_codec.cpp index 94e6984df..76a8f7d02 100644 --- a/src/tada_codec.cpp +++ b/src/tada_codec.cpp @@ -1052,7 +1052,7 @@ void tada_codec_free(struct tada_codec_context* ctx) { if (ctx->ctx_perm) ggml_free(ctx->ctx_perm); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->owns_backend) { diff --git a/src/tada_encoder.cpp b/src/tada_encoder.cpp index bcf332418..db8fc89bf 100644 --- a/src/tada_encoder.cpp +++ b/src/tada_encoder.cpp @@ -746,7 +746,7 @@ void tada_encoder_free(tada_encoder_context* ctx) { if (ctx->ctx_inv) ggml_free(ctx->ctx_inv); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend) diff --git a/src/tada_tts.cpp b/src/tada_tts.cpp index 691246882..5ce6b35cb 100644 --- a/src/tada_tts.cpp +++ b/src/tada_tts.cpp @@ -2332,7 +2332,7 @@ int tada_load_prompt(struct tada_context* ctx, const char* path) { // Clean up if (wl.buf) - ggml_backend_buffer_free(wl.buf); + core_gguf::release_weight_buffer(wl.buf); if (wl.ctx) ggml_free(wl.ctx); @@ -3583,7 +3583,7 @@ void tada_free(struct tada_context* ctx) { if (ctx->kv_ctx) ggml_free(ctx->kv_ctx); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/titanet.cpp b/src/titanet.cpp index 38ba19e02..0a412ca5a 100644 --- a/src/titanet.cpp +++ b/src/titanet.cpp @@ -688,7 +688,7 @@ extern "C" void titanet_free(struct titanet_context* ctx) { if (ctx->weight_ctx) ggml_free(ctx->weight_ctx); if (ctx->buf) - ggml_backend_buffer_free(ctx->buf); + core_gguf::release_weight_buffer(ctx->buf); if (ctx->backend) ggml_backend_free(ctx->backend); delete ctx; diff --git a/src/vibevoice.cpp b/src/vibevoice.cpp index 4817ae8a2..0ff40ec88 100644 --- a/src/vibevoice.cpp +++ b/src/vibevoice.cpp @@ -473,13 +473,13 @@ extern "C" void vibevoice_free(struct vibevoice_context* ctx) { if (ctx->ctx_perm) ggml_free(ctx->ctx_perm); if (ctx->voice.buf) - ggml_backend_buffer_free(ctx->voice.buf); + core_gguf::release_weight_buffer(ctx->voice.buf); if (ctx->voice.ctx) ggml_free(ctx->voice.ctx); if (ctx->buf) - ggml_backend_buffer_free(ctx->buf); + core_gguf::release_weight_buffer(ctx->buf); if (ctx->buf_cpu) - ggml_backend_buffer_free(ctx->buf_cpu); + core_gguf::release_weight_buffer(ctx->buf_cpu); if (ctx->weight_ctx) ggml_free(ctx->weight_ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/voxcpm2_tts.cpp b/src/voxcpm2_tts.cpp index 23369a1f6..eae94ac8f 100644 --- a/src/voxcpm2_tts.cpp +++ b/src/voxcpm2_tts.cpp @@ -6839,7 +6839,7 @@ void voxcpm2_free(struct voxcpm2_context* ctx) { ctx->gpu_ggml_ctx = nullptr; } if (ctx->weight_buf) { - ggml_backend_buffer_free(ctx->weight_buf); + core_gguf::release_weight_buffer(ctx->weight_buf); ctx->weight_buf = nullptr; } if (ctx->ggml_ctx) { diff --git a/src/voxtral.cpp b/src/voxtral.cpp index 0ae27065d..0765c2360 100644 --- a/src/voxtral.cpp +++ b/src/voxtral.cpp @@ -962,9 +962,9 @@ extern "C" void voxtral_free(voxtral_context* ctx) { if (ctx->fused_ctx) ggml_free(ctx->fused_ctx); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.buf_cpu) - ggml_backend_buffer_free(ctx->model.buf_cpu); + core_gguf::release_weight_buffer(ctx->model.buf_cpu); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu) diff --git a/src/voxtral4b.cpp b/src/voxtral4b.cpp index f917694c1..909a2507c 100644 --- a/src/voxtral4b.cpp +++ b/src/voxtral4b.cpp @@ -1070,9 +1070,9 @@ extern "C" void voxtral4b_free(voxtral4b_context* ctx) { if (ctx->fused_ctx) ggml_free(ctx->fused_ctx); if (ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->model.buf_cpu) - ggml_backend_buffer_free(ctx->model.buf_cpu); + core_gguf::release_weight_buffer(ctx->model.buf_cpu); if (ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/voxtral_tts.cpp b/src/voxtral_tts.cpp index 518a9f38e..3e512c674 100644 --- a/src/voxtral_tts.cpp +++ b/src/voxtral_tts.cpp @@ -1814,7 +1814,7 @@ extern "C" void voxtral_tts_free(voxtral_tts_context* ctx) { if (ctx->kv_ctx) ggml_free(ctx->kv_ctx); if (ctx->buf) - ggml_backend_buffer_free(ctx->buf); + core_gguf::release_weight_buffer(ctx->buf); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend && ctx->backend != ctx->backend_cpu) diff --git a/src/wav2vec2-ggml.h b/src/wav2vec2-ggml.h index 0dbe8c16d..6104faea6 100755 --- a/src/wav2vec2-ggml.h +++ b/src/wav2vec2-ggml.h @@ -15,6 +15,8 @@ #pragma once +#include "core/gguf_loader.h" + #include "ggml.h" #include "ggml-backend.h" #include "gguf.h" @@ -116,9 +118,12 @@ struct wav2vec2_model { // Free in dependency order: buf (depends on backend) → ctx → backend. // Without this, on Metal the residency set survives past main() and // ggml_metal's static teardown trips ggml_metal_rsets_free's assert. + // + // `buf` came from core_gguf::load_weights, so on a zero-copy backend it + // carries a host mmap the backend does not own; release_weight_buffer + // frees the buffer and then unmaps that region. ~wav2vec2_model() noexcept { - if (buf) - ggml_backend_buffer_free(buf); + core_gguf::release_weight_buffer(buf); if (ctx) ggml_free(ctx); if (backend) diff --git a/src/wespeaker.cpp b/src/wespeaker.cpp index a6bfde248..00ad2fce2 100644 --- a/src/wespeaker.cpp +++ b/src/wespeaker.cpp @@ -564,7 +564,7 @@ extern "C" void wespeaker_free(struct wespeaker_context* ctx) { if (ctx->sched) ggml_backend_sched_free(ctx->sched); if (ctx->owns_model && ctx->model.buf) - ggml_backend_buffer_free(ctx->model.buf); + core_gguf::release_weight_buffer(ctx->model.buf); if (ctx->owns_model && ctx->model.ctx) ggml_free(ctx->model.ctx); if (ctx->backend_cpu && ctx->backend_cpu != ctx->backend) diff --git a/src/zonos_tts.cpp b/src/zonos_tts.cpp index 9cf35cc0e..c43332666 100644 --- a/src/zonos_tts.cpp +++ b/src/zonos_tts.cpp @@ -2718,7 +2718,7 @@ void zonos_tts_free(struct zonos_tts_context* ctx) { if (ctx->dac_ctx_perm) ggml_free(ctx->dac_ctx_perm); if (ctx->dac_buf_w) - ggml_backend_buffer_free(ctx->dac_buf_w); + core_gguf::release_weight_buffer(ctx->dac_buf_w); if (ctx->dac_ctx_w) ggml_free(ctx->dac_ctx_w); if (ctx->kv_buf) @@ -2726,7 +2726,7 @@ void zonos_tts_free(struct zonos_tts_context* ctx) { if (ctx->kv_ctx) ggml_free(ctx->kv_ctx); if (ctx->buf_w) - ggml_backend_buffer_free(ctx->buf_w); + core_gguf::release_weight_buffer(ctx->buf_w); if (ctx->ctx_w) ggml_free(ctx->ctx_w); if (ctx->backend_cpu) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f3430aefb..ab7395f88 100755 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -467,6 +467,68 @@ catch_discover_tests(test-gguf-bounds PROPERTIES LABELS "unit" ) +# ─── test-gguf-release — contract of core_gguf::release_weight_buffer ────────── +# The entry point that frees a weight buffer AND unmaps the host region behind +# it. These cases need no GPU: the CPU mmap path, the legacy alloc+copy path +# (which registers no mapping and must still be released cleanly), repeat calls +# and the null handle. +add_executable(test-gguf-release test-gguf-release.cpp) +target_include_directories(test-gguf-release PRIVATE + ${PROJECT_SOURCE_DIR}/src + ${PROJECT_SOURCE_DIR}/ggml/include +) +target_link_libraries(test-gguf-release PRIVATE + Catch2::Catch2WithMain + crispasr-lib + ggml +) +catch_discover_tests(test-gguf-release + TEST_SPEC "[unit]" + PROPERTIES LABELS "unit" +) + +# ─── test-gguf-mapping-released — the weight mmap must not outlive the model ─── +# Asks the kernel which regions still name the weight file after the weights +# are freed. Exact rather than a footprint threshold, so there is nothing to +# settle and nothing to poll. The zero-copy leg self-skips where no device +# advertises buffer_from_host_ptr. +add_executable(test-gguf-mapping-released test-gguf-mapping-released.cpp) +target_include_directories(test-gguf-mapping-released PRIVATE + ${PROJECT_SOURCE_DIR}/src + ${PROJECT_SOURCE_DIR}/ggml/include +) +target_link_libraries(test-gguf-mapping-released PRIVATE + Catch2::Catch2WithMain + crispasr-lib + ggml +) +catch_discover_tests(test-gguf-mapping-released + TEST_SPEC "[unit]" + PROPERTIES LABELS "unit" +) + +# ─── test-crisp-audio-mapping-released — the same oracle, one step out ───────── +# crisp_audio takes its weight buffer from core_gguf::load_weights and owns it +# from there, so the release has to happen in crisp_audio_free(). Drives the +# public C API on a synthetic one-layer tower and asks the kernel which regions +# still name the GGUF. The GPU legs self-skip where no device advertises +# buffer_from_host_ptr; the CPU leg runs everywhere. +add_executable(test-crisp-audio-mapping-released test-crisp-audio-mapping-released.cpp) +target_include_directories(test-crisp-audio-mapping-released PRIVATE + ${PROJECT_SOURCE_DIR}/src + ${PROJECT_SOURCE_DIR}/ggml/include +) +target_link_libraries(test-crisp-audio-mapping-released PRIVATE + Catch2::Catch2WithMain + crisp_audio + crispasr-lib + ggml +) +catch_discover_tests(test-crisp-audio-mapping-released + TEST_SPEC "[unit]" + PROPERTIES LABELS "unit" +) + # ─── test-gguf-split-alloc — regression guard for issue #276 ─────────────────── # Verifies load_weights_split() correctly partitions tensors across GPU/CPU # backend buffers, respects the is_gpu predicate, and that free_weights cleans diff --git a/tests/test-crisp-audio-mapping-released.cpp b/tests/test-crisp-audio-mapping-released.cpp new file mode 100644 index 000000000..7456abc79 --- /dev/null +++ b/tests/test-crisp-audio-mapping-released.cpp @@ -0,0 +1,310 @@ +// test-crisp-audio-mapping-released.cpp — crisp_audio must not outlive its +// weight mapping either. +// +// crisp_audio's load_model() takes its backend buffer from +// core_gguf::load_weights and moves it into crisp_audio_context::model_buf. +// crisp_audio_free() destroyed that handle with ggml_backend_buffer_free(), +// which on a device advertising `buffer_from_host_ptr` releases the device-side +// view and leaves the host mmap in place — the loader's side-map entry is never +// taken and the pages are never unmapped. Two shipped consumers reach this +// path: src/qwen3_asr.cpp and src/higgs_stt.cpp both link crisp_audio, and +// CrispEmbed's BidirLM-Omni audio path links the same library. +// +// test-gguf-mapping-released.cpp pins the loader's own release. This file pins +// the consumer: that crisp_audio's handle actually reaches +// core_gguf::release_weight_buffer, through the public C API rather than +// through the loader. +// +// The oracle is the same exact one — after the free, no region of this process +// may name the GGUF. Which leg pins what: +// +// * The GPU leg is the one that fails without the fix. It self-skips where no +// device advertises buffer_from_host_ptr, because the leaking path does not +// exist there. +// * The CPU leg cannot fail on this bug — the CPU mmap path unmaps through +// the buffer's own free callback whichever entry point releases it. It is +// here so the case is not a pure skip on Linux/Windows CI, and it does +// guard the weaker claim that crisp_audio releases at all. +// +// No model file, no download: a synthetic one-layer tower reaches the same +// loader branch a multi-gigabyte encoder does. + +#include + +#include "test-region-probe.h" + +#include "crisp_audio.h" + +#include "core/gguf_loader.h" + +#include "ggml-backend.h" +#include "ggml.h" +#include "gguf.h" + +#include +#include +#include +#include + +namespace { + +using test_region::absolute_path_of; +using test_region::count_regions_backed_by; +using test_region::region_probe_available; + +// Portable env helper (Windows has no POSIX setenv). +void test_setenv(const char* k, const char* v) { +#if defined(_WIN32) + _putenv_s(k, v); +#else + ::setenv(k, v, 1); +#endif +} + +// The dimensions the fixture declares. Small enough that the positional +// embedding crisp_audio precomputes at init costs nothing; d_model must stay +// above 2, since the sinusoid divides by (d_model/2 - 1). +constexpr uint32_t kLayers = 1; +constexpr uint32_t kDModel = 8; + +// Every tensor load_model() looks up by name. Listed rather than derived: a +// rename in audio_tower.cpp should fail this fixture loudly at load time, not +// leave it silently loading a tower with no weights. +const char* kTowerTensors[] = { + "audio.conv.1.weight", "audio.conv.1.bias", "audio.conv.2.weight", "audio.conv.2.bias", + "audio.conv.3.weight", "audio.conv.3.bias", "audio.conv_out.weight", "audio.conv_out.bias", + "audio.ln_post.weight", "audio.ln_post.bias", "audio.proj1.weight", "audio.proj1.bias", + "audio.proj2.weight", "audio.proj2.bias", +}; + +const char* kBlockSuffixes[] = { + "attn_norm.weight", "attn_norm.bias", "attn_q.weight", "attn_q.bias", "attn_k.weight", "attn_k.bias", + "attn_v.weight", "attn_v.bias", "attn_out.weight", "attn_out.bias", "ffn_norm.weight", "ffn_norm.bias", + "ffn_up.weight", "ffn_up.bias", "ffn_down.weight", "ffn_down.bias", +}; + +// Weights wide enough that the mapping is a region of its own rather than +// something the kernel might fold into a neighbour. One tensor carries the +// bulk; the rest only need to exist under the right name. +constexpr int kBulkElems = 1 << 20; // 4 MiB of f32 +constexpr int kSmallElems = 4; + +void add_tensor(ggml_context* ctx, gguf_context* g, const char* name, int elems) { + ggml_tensor* t = ggml_new_tensor_1d(ctx, GGML_TYPE_F32, elems); + REQUIRE(t != nullptr); + ggml_set_name(t, name); + float* d = (float*)t->data; + for (int i = 0; i < elems; i++) + d[i] = (float)i; + gguf_add_tensor(g, t); +} + +// A GGUF crisp_audio loads successfully: hparams under the default +// `crisp_audio.` key prefix plus the full tensor set for a one-layer tower. +void write_tower_gguf(const std::string& path) { + const size_t n_tensors = std::size(kTowerTensors) + kLayers * std::size(kBlockSuffixes); + const size_t mem = (size_t)kBulkElems * sizeof(float) + + n_tensors * ((size_t)kSmallElems * sizeof(float) + ggml_tensor_overhead()) + 8192; + ggml_init_params ip = {/*mem_size=*/mem, /*mem_buffer=*/nullptr, /*no_alloc=*/false}; + ggml_context* ctx = ggml_init(ip); + REQUIRE(ctx != nullptr); + + gguf_context* g = gguf_init_empty(); + gguf_set_val_str(g, "general.architecture", "crisp_audio_test"); + // d_model is the key load_model probes to decide the metadata prefix, so + // it has to be present for the rest of these to be read at all. + gguf_set_val_u32(g, "crisp_audio.d_model", kDModel); + gguf_set_val_u32(g, "crisp_audio.n_layers", kLayers); + gguf_set_val_u32(g, "crisp_audio.n_heads", 1); + gguf_set_val_u32(g, "crisp_audio.head_dim", kDModel); + gguf_set_val_u32(g, "crisp_audio.ff_dim", kDModel); + gguf_set_val_u32(g, "crisp_audio.conv_channels", kDModel); + gguf_set_val_u32(g, "crisp_audio.max_source_pos", 8); + gguf_set_val_u32(g, "crisp_audio.output_dim", kDModel); + + bool bulk_used = false; + for (const char* name : kTowerTensors) { + add_tensor(ctx, g, name, bulk_used ? kSmallElems : kBulkElems); + bulk_used = true; + } + for (uint32_t i = 0; i < kLayers; i++) { + for (const char* suffix : kBlockSuffixes) { + char name[160]; + std::snprintf(name, sizeof(name), "audio.blk.%u.%s", i, suffix); + add_tensor(ctx, g, name, kSmallElems); + } + } + + REQUIRE(gguf_write_to_file(g, path.c_str(), /*only_meta=*/false)); + gguf_free(g); + ggml_free(ctx); +} + +// A GGUF the loader maps but crisp_audio then rejects: valid hparams, no tower +// tensors. load_model() returns false after load_weights() has already mapped +// the file, and crisp_audio_init_from_file cleans up through crisp_audio_free. +void write_rejected_gguf(const std::string& path) { + const size_t mem = (size_t)kBulkElems * sizeof(float) + ggml_tensor_overhead() + 4096; + ggml_init_params ip = {/*mem_size=*/mem, /*mem_buffer=*/nullptr, /*no_alloc=*/false}; + ggml_context* ctx = ggml_init(ip); + REQUIRE(ctx != nullptr); + + gguf_context* g = gguf_init_empty(); + gguf_set_val_str(g, "general.architecture", "crisp_audio_test"); + gguf_set_val_u32(g, "crisp_audio.d_model", kDModel); + gguf_set_val_u32(g, "crisp_audio.n_layers", kLayers); + gguf_set_val_u32(g, "crisp_audio.max_source_pos", 8); + add_tensor(ctx, g, "audio.not_a_tower_tensor", kBulkElems); + + REQUIRE(gguf_write_to_file(g, path.c_str(), /*only_meta=*/false)); + gguf_free(g); + ggml_free(ctx); +} + +struct Fixture { + std::string rel; + std::string abs; + Fixture(const char* name, bool loadable) : rel(name) { + if (loadable) + write_tower_gguf(rel); + else + write_rejected_gguf(rel); + abs = absolute_path_of(rel); + } + ~Fixture() { std::remove(rel.c_str()); } + Fixture(const Fixture&) = delete; + Fixture& operator=(const Fixture&) = delete; +}; + +// True when this machine has the GPU device crisp_audio_init_from_file would +// pick AND that device hands host pointers to the backend — the two conditions +// that together select the leaking loader branch. +bool host_ptr_gpu_available() { + ggml_backend_dev_t dev = ggml_backend_dev_by_type(GGML_BACKEND_DEVICE_TYPE_GPU); + if (!dev) + return false; + ggml_backend_dev_props props{}; + ggml_backend_dev_get_props(dev, &props); + return props.caps.buffer_from_host_ptr; +} + +crisp_audio_params quiet_params(bool use_gpu) { + crisp_audio_params p = crisp_audio_params_default(); + p.verbosity = 0; + p.use_gpu = use_gpu; + return p; +} + +} // namespace + +TEST_CASE("crisp_audio_free unmaps the zero-copy GPU path's weight region", "[unit][crisp-audio-mapping]") { + if (!region_probe_available()) { + SUCCEED("region enumeration unavailable on this platform"); + return; + } + if (!host_ptr_gpu_available()) { + SUCCEED("no GPU device advertising buffer_from_host_ptr — the leaking path does not exist here"); + return; + } + test_setenv("CRISPASR_GGUF_MMAP", "1"); + + Fixture fx("crispasr_test_crisp_audio_gpu.gguf", /*loadable=*/true); + REQUIRE(count_regions_backed_by(fx.abs) == 0); + + crisp_audio_params p = quiet_params(/*use_gpu=*/true); + crisp_audio_context* ctx = crisp_audio_init_from_file(fx.rel.c_str(), &p); + REQUIRE(ctx != nullptr); + // Positive control: crisp_audio really took the zero-copy branch. Without + // it a fall-through to the legacy alloc+copy loader would satisfy the + // absence assertion below having never created the mapping under test. + // Not pinned to 1 — a kernel may report one mapping as several adjacent + // regions; the repeated-cycles case is what pins accumulation. + REQUIRE(count_regions_backed_by(fx.abs) >= 1); + + crisp_audio_free(ctx); + REQUIRE(count_regions_backed_by(fx.abs) == 0); +} + +TEST_CASE("repeated crisp_audio init/free cycles leave no mapping behind", "[unit][crisp-audio-mapping]") { + if (!region_probe_available()) { + SUCCEED("region enumeration unavailable on this platform"); + return; + } + if (!host_ptr_gpu_available()) { + SUCCEED("no GPU device advertising buffer_from_host_ptr — the leaking path does not exist here"); + return; + } + test_setenv("CRISPASR_GGUF_MMAP", "1"); + + // Each init maps the file again and records a separate region, so a + // release that handled only one of them accumulates the rest. Five cycles + // make that a count of five rather than an ambiguous one. + Fixture fx("crispasr_test_crisp_audio_loop.gguf", /*loadable=*/true); + crisp_audio_params p = quiet_params(/*use_gpu=*/true); + + for (int i = 0; i < 5; i++) { + crisp_audio_context* ctx = crisp_audio_init_from_file(fx.rel.c_str(), &p); + REQUIRE(ctx != nullptr); + crisp_audio_free(ctx); + } + REQUIRE(count_regions_backed_by(fx.abs) == 0); +} + +TEST_CASE("a crisp_audio load rejected after mapping leaves no region", "[unit][crisp-audio-mapping]") { + if (!region_probe_available()) { + SUCCEED("region enumeration unavailable on this platform"); + return; + } + if (!host_ptr_gpu_available()) { + SUCCEED("no GPU device advertising buffer_from_host_ptr — the leaking path does not exist here"); + return; + } + test_setenv("CRISPASR_GGUF_MMAP", "1"); + + // load_weights succeeds and maps the file; the missing tower tensors are + // what fail, one step later. crisp_audio_init_from_file's own cleanup call + // is the release path here, and it is the same one the success case uses. + Fixture fx("crispasr_test_crisp_audio_reject.gguf", /*loadable=*/false); + REQUIRE(count_regions_backed_by(fx.abs) == 0); + + crisp_audio_params p = quiet_params(/*use_gpu=*/true); + REQUIRE(crisp_audio_init_from_file(fx.rel.c_str(), &p) == nullptr); + REQUIRE(count_regions_backed_by(fx.abs) == 0); + + // Positive control, after the fact: the same file on the same backend does + // map. Without it the zero above would also be satisfied by a loader that + // rejected the GGUF before ever mapping it. + ggml_backend_dev_t dev = ggml_backend_dev_by_type(GGML_BACKEND_DEVICE_TYPE_GPU); + REQUIRE(dev != nullptr); + ggml_backend_t backend = ggml_backend_dev_init(dev, nullptr); + REQUIRE(backend != nullptr); + core_gguf::WeightLoad wl; + REQUIRE(core_gguf::load_weights(fx.rel.c_str(), backend, "test-crisp-audio-reject", wl)); + REQUIRE(count_regions_backed_by(fx.abs) >= 1); + core_gguf::free_weights(wl); + REQUIRE(count_regions_backed_by(fx.abs) == 0); + ggml_backend_free(backend); +} + +TEST_CASE("crisp_audio_free unmaps the CPU mmap path's weight region", "[unit][crisp-audio-mapping]") { + if (!region_probe_available()) { + SUCCEED("region enumeration unavailable on this platform"); + return; + } + test_setenv("CRISPASR_GGUF_MMAP", "1"); + + // This leg passed before the fix too — the CPU mmap path unmaps through + // the buffer's own free callback whichever entry point releases it. It + // runs everywhere, and it is what keeps this file from being a pure skip + // on hosts without a host-pointer GPU. + Fixture fx("crispasr_test_crisp_audio_cpu.gguf", /*loadable=*/true); + REQUIRE(count_regions_backed_by(fx.abs) == 0); + + crisp_audio_params p = quiet_params(/*use_gpu=*/false); + crisp_audio_context* ctx = crisp_audio_init_from_file(fx.rel.c_str(), &p); + REQUIRE(ctx != nullptr); + REQUIRE(count_regions_backed_by(fx.abs) >= 1); + + crisp_audio_free(ctx); + REQUIRE(count_regions_backed_by(fx.abs) == 0); +} diff --git a/tests/test-gguf-bounds.cpp b/tests/test-gguf-bounds.cpp index 7a7d4a862..ac6ac470b 100644 --- a/tests/test-gguf-bounds.cpp +++ b/tests/test-gguf-bounds.cpp @@ -15,6 +15,8 @@ #include +#include "test-region-probe.h" + #include "core/gguf_loader.h" #include "ggml-backend.h" @@ -23,6 +25,7 @@ #include "gguf.h" #include +#include #include #include // truncate @@ -47,6 +50,45 @@ void write_valid_gguf(const std::string& path, int n) { ggml_free(ctx); } +// Portable env helper (Windows has no POSIX setenv). +void test_setenv(const char* k, const char* v) { +#if defined(_WIN32) + _putenv_s(k, v); +#else + ::setenv(k, v, 1); +#endif +} + +// Copy `src` and truncate the copy 8 bytes short of the declared tensor data, +// so metadata parses fully but the tensor overruns the file. +void write_truncated_copy(const std::string& src_path, const std::string& dst_path, size_t keep_bytes) { + FILE* src = std::fopen(src_path.c_str(), "rb"); + FILE* dst = std::fopen(dst_path.c_str(), "wb"); + REQUIRE(src); + REQUIRE(dst); + char buf[4096]; + size_t r; + while ((r = std::fread(buf, 1, sizeof(buf), src)) > 0) + std::fwrite(buf, 1, r, dst); + std::fclose(src); + std::fclose(dst); + REQUIRE(::truncate(dst_path.c_str(), (off_t)keep_bytes) == 0); +} + +// A GPU backend that hands host pointers to the device, or nullptr when this +// machine has none. That capability selects the zero-copy load path, which is +// the only one that maps the file into a buffer the backend does not own. +ggml_backend_t init_host_ptr_gpu_backend() { + ggml_backend_dev_t dev = ggml_backend_dev_by_type(GGML_BACKEND_DEVICE_TYPE_GPU); + if (!dev) + return nullptr; + ggml_backend_dev_props props{}; + ggml_backend_dev_get_props(dev, &props); + if (!props.caps.buffer_from_host_ptr) + return nullptr; + return ggml_backend_dev_init(dev, nullptr); +} + // Re-read the written file to get the true data-section offset (the write // context's gguf_get_data_offset is not populated until write). size_t read_data_offset(const std::string& path) { @@ -77,6 +119,7 @@ TEST_CASE("core_gguf::load_weights rejects a truncated GGUF without crashing", " core_gguf::WeightLoad wl; REQUIRE(core_gguf::load_weights(good.c_str(), backend, "test-ok", wl)); REQUIRE(wl.tensors.count("test.weight") == 1); + core_gguf::free_weights(wl); } // Craft the malicious file: copy the good one, then truncate to just SHORT @@ -84,19 +127,7 @@ TEST_CASE("core_gguf::load_weights rejects a truncated GGUF without crashing", " // tensor overruns the file by 8 bytes). This forces control into the // subtractive bounds check in load_weights (nbytes > size - data_off - off), // not the earlier magic/metadata rejection — i.e. the exact hardened path. - { - FILE* src = std::fopen(good.c_str(), "rb"); - FILE* dst = std::fopen(bad.c_str(), "wb"); - REQUIRE(src); - REQUIRE(dst); - char buf[4096]; - size_t r; - while ((r = std::fread(buf, 1, sizeof(buf), src)) > 0) - std::fwrite(buf, 1, r, dst); - std::fclose(src); - std::fclose(dst); - REQUIRE(::truncate(bad.c_str(), (off_t)(data_off + nbytes - 8)) == 0); - } + write_truncated_copy(good, bad, data_off + nbytes - 8); // The load must fail gracefully (false), not SIGBUS. Reaching this REQUIRE at // all means no crash; the value check confirms it was rejected. @@ -109,3 +140,42 @@ TEST_CASE("core_gguf::load_weights rejects a truncated GGUF without crashing", " std::remove(bad.c_str()); ggml_backend_free(backend); } + +TEST_CASE("a rejected GGUF leaves no mapping behind on the zero-copy path", "[unit][gguf-bounds]") { + // The zero-copy path maps the whole file and hands it to the device before + // it validates tensor bounds, so the rejection this file's first case + // covers happens with a mapping already registered against the backend + // buffer. Returning false there looks safe and is not: without an explicit + // release, both the buffer and the whole-file mapping are abandoned — and + // this branch is reached by a truncated or crafted GGUF, which is + // attacker-supplied input, so the leak is reachable on demand. + if (!test_region::region_probe_available()) { + SUCCEED("region enumeration unavailable on this platform"); + return; + } + test_setenv("CRISPASR_GGUF_MMAP", "1"); + + ggml_backend_t backend = init_host_ptr_gpu_backend(); + if (!backend) { + SUCCEED("no GPU device advertising buffer_from_host_ptr — this path does not exist here"); + return; + } + + const std::string good = "crispasr_test_gguf_reject_ok.gguf"; + const std::string bad = "crispasr_test_gguf_reject_trunc.gguf"; + const int n = 65536; // 256 KiB of tensor data + write_valid_gguf(good, n); + const size_t data_off = read_data_offset(good); + write_truncated_copy(good, bad, data_off + (size_t)n * sizeof(float) - 8); + + const std::string bad_abs = test_region::absolute_path_of(bad); + REQUIRE(test_region::count_regions_backed_by(bad_abs) == 0); + + core_gguf::WeightLoad wl; + REQUIRE_FALSE(core_gguf::load_weights(bad.c_str(), backend, "test-reject", wl)); + REQUIRE(test_region::count_regions_backed_by(bad_abs) == 0); + + std::remove(good.c_str()); + std::remove(bad.c_str()); + ggml_backend_free(backend); +} diff --git a/tests/test-gguf-mapping-released.cpp b/tests/test-gguf-mapping-released.cpp new file mode 100644 index 000000000..2a9df5b08 --- /dev/null +++ b/tests/test-gguf-mapping-released.cpp @@ -0,0 +1,183 @@ +// test-gguf-mapping-released.cpp — the weight mmap must be gone after the +// weights are freed. +// +// core_gguf::load_weights maps the whole GGUF and hands the region to the +// backend. On a device advertising `buffer_from_host_ptr` (Apple-Silicon +// Metal) the backend does not own those pages — `buffer_from_host_ptr` has no +// deallocator parameter, and Metal passes `deallocator:nil` — so freeing the +// backend buffer alone left the file mapped for the life of the process. The +// mapping is `MAP_PRIVATE | PROT_READ|PROT_WRITE`, so every page privatizes on +// first read: the resident pages are dirty and anonymous, and can only be +// compressed or swapped, never dropped. A process that loaded several models +// therefore held all of them at once. +// +// The oracle is exact rather than a footprint threshold: after the free, no +// region of this process may name the weight file. There is nothing to settle +// and nothing to poll, and the assertion survives a change of release +// mechanism because its subject is the mapping, not munmap. +// +// Two paths are covered. The CPU mmap path runs everywhere and releases +// through the buffer's own free callback. The zero-copy GPU path is the one +// that leaked; it self-skips where no device advertises the capability. + +#include + +#include "test-region-probe.h" + +#include "core/gguf_loader.h" + +#include "ggml-backend.h" +#include "ggml-cpu.h" +#include "ggml.h" +#include "gguf.h" + +#include +#include +#include + +namespace { + +// Portable env helper (Windows has no POSIX setenv). +void test_setenv(const char* k, const char* v) { +#if defined(_WIN32) + _putenv_s(k, v); +#else + ::setenv(k, v, 1); +#endif +} + +using test_region::absolute_path_of; +using test_region::count_regions_backed_by; +using test_region::region_probe_available; + +// Write a GGUF several megabytes wide, so its mapping is a region of its own +// rather than something the kernel might fold into a neighbour. +void write_gguf(const std::string& path, int n_tensors, int elems) { + const size_t mem = (size_t)n_tensors * ((size_t)elems * sizeof(float) + ggml_tensor_overhead()) + 4096; + ggml_init_params ip = {/*mem_size=*/mem, /*mem_buffer=*/nullptr, /*no_alloc=*/false}; + ggml_context* ctx = ggml_init(ip); + REQUIRE(ctx != nullptr); + + gguf_context* g = gguf_init_empty(); + gguf_set_val_str(g, "general.architecture", "test_release"); + for (int i = 0; i < n_tensors; i++) { + char name[64]; + snprintf(name, sizeof(name), "blk.%d.weight", i); + ggml_tensor* t = ggml_new_tensor_1d(ctx, GGML_TYPE_F32, elems); + ggml_set_name(t, name); + float* d = (float*)t->data; + for (int j = 0; j < elems; j++) + d[j] = (float)(i + 1); + gguf_add_tensor(g, t); + } + REQUIRE(gguf_write_to_file(g, path.c_str(), /*only_meta=*/false)); + gguf_free(g); + ggml_free(ctx); +} + +// A GPU backend that hands host pointers to the device, or nullptr when this +// machine has none. That capability is exactly what selects the leaking path. +ggml_backend_t init_host_ptr_gpu_backend() { + ggml_backend_dev_t dev = ggml_backend_dev_by_type(GGML_BACKEND_DEVICE_TYPE_GPU); + if (!dev) + return nullptr; + ggml_backend_dev_props props{}; + ggml_backend_dev_get_props(dev, &props); + if (!props.caps.buffer_from_host_ptr) + return nullptr; + return ggml_backend_dev_init(dev, nullptr); +} + +struct Fixture { + std::string rel; + std::string abs; + explicit Fixture(const char* name) : rel(name) { + write_gguf(rel, /*n_tensors=*/4, /*elems=*/262144); // 4 MiB of weights + abs = absolute_path_of(rel); + } + ~Fixture() { std::remove(rel.c_str()); } + Fixture(const Fixture&) = delete; + Fixture& operator=(const Fixture&) = delete; +}; + +} // namespace + +TEST_CASE("freeing weights unmaps the CPU mmap path's region", "[unit][gguf-mapping]") { + if (!region_probe_available()) { + SUCCEED("region enumeration unavailable on this platform"); + return; + } + test_setenv("CRISPASR_GGUF_MMAP", "1"); + + ggml_backend_t backend = ggml_backend_cpu_init(); + REQUIRE(backend != nullptr); + Fixture fx("crispasr_test_mapping_cpu.gguf"); + REQUIRE(count_regions_backed_by(fx.abs) == 0); + + core_gguf::WeightLoad wl; + REQUIRE(core_gguf::load_weights(fx.rel.c_str(), backend, "test-map-cpu", wl)); + // Positive control: the mmap path was taken and the file is mapped. The + // count is not pinned to 1 because a kernel may report one mapping as + // several adjacent regions; what this case asserts exactly is the zero + // below, and the repeated-cycles case asserts the per-load accumulation. + REQUIRE(count_regions_backed_by(fx.abs) >= 1); + + core_gguf::free_weights(wl); + REQUIRE(count_regions_backed_by(fx.abs) == 0); + + ggml_backend_free(backend); +} + +TEST_CASE("freeing weights unmaps the zero-copy GPU path's region", "[unit][gguf-mapping]") { + if (!region_probe_available()) { + SUCCEED("region enumeration unavailable on this platform"); + return; + } + test_setenv("CRISPASR_GGUF_MMAP", "1"); + + ggml_backend_t backend = init_host_ptr_gpu_backend(); + if (!backend) { + SUCCEED("no GPU device advertising buffer_from_host_ptr — the leaking path does not exist here"); + return; + } + Fixture fx("crispasr_test_mapping_gpu.gguf"); + REQUIRE(count_regions_backed_by(fx.abs) == 0); + + core_gguf::WeightLoad wl; + REQUIRE(core_gguf::load_weights(fx.rel.c_str(), backend, "test-map-gpu", wl)); + // Positive control: the zero-copy path was actually taken. Without it a + // fall-through to the legacy alloc+copy loader would satisfy the absence + // assertion below having never created the mapping under test. Not pinned + // to 1 — see the CPU case. + REQUIRE(count_regions_backed_by(fx.abs) >= 1); + + core_gguf::free_weights(wl); + REQUIRE(count_regions_backed_by(fx.abs) == 0); + + ggml_backend_free(backend); +} + +TEST_CASE("repeated load/free cycles leave no mapping behind", "[unit][gguf-mapping]") { + if (!region_probe_available()) { + SUCCEED("region enumeration unavailable on this platform"); + return; + } + test_setenv("CRISPASR_GGUF_MMAP", "1"); + + // Each load maps the file again and records a separate region, so a + // release that handled only one of them accumulates the rest. Twenty + // cycles make that a count of twenty rather than an ambiguous one. + ggml_backend_t gpu = init_host_ptr_gpu_backend(); + ggml_backend_t backend = gpu ? gpu : ggml_backend_cpu_init(); + REQUIRE(backend != nullptr); + Fixture fx("crispasr_test_mapping_loop.gguf"); + + for (int i = 0; i < 20; i++) { + core_gguf::WeightLoad wl; + REQUIRE(core_gguf::load_weights(fx.rel.c_str(), backend, "test-map-loop", wl)); + core_gguf::free_weights(wl); + } + REQUIRE(count_regions_backed_by(fx.abs) == 0); + + ggml_backend_free(backend); +} diff --git a/tests/test-gguf-release.cpp b/tests/test-gguf-release.cpp new file mode 100644 index 000000000..2cc3886ca --- /dev/null +++ b/tests/test-gguf-release.cpp @@ -0,0 +1,165 @@ +// test-gguf-release.cpp — contract of core_gguf::release_weight_buffer(). +// +// load_weights() can hand back a backend buffer that owns a host mmap the +// backend itself does not own: on a device advertising `buffer_from_host_ptr` +// (Apple-Silicon Metal) the weight file is mapped and passed to +// `newBufferWithBytesNoCopy:…deallocator:nil`, so ggml_backend_buffer_free() +// releases the device-side view and leaves the mapping behind. +// release_weight_buffer() is the entry point that releases both. +// +// This file pins the entry point's contract on paths that need no GPU, so it +// runs everywhere: the CPU mmap path, the legacy alloc+copy path (which +// registers no mapping at all, and must therefore be an ordinary buffer free), +// repeat calls, and the null handle. test-gguf-mapping-released.cpp covers the +// mapping's actual disappearance. + +#include + +#include "core/gguf_loader.h" + +#include "ggml-backend.h" +#include "ggml-cpu.h" +#include "ggml.h" +#include "gguf.h" + +#include +#include +#include + +namespace { + +// Write a minimal valid GGUF with one F32 tensor of `n` elements. +void write_gguf(const std::string& path, int n) { + ggml_init_params ip = {/*mem_size=*/(size_t)n * sizeof(float) + ggml_tensor_overhead() + 1024, + /*mem_buffer=*/nullptr, /*no_alloc=*/false}; + ggml_context* ctx = ggml_init(ip); + REQUIRE(ctx != nullptr); + ggml_tensor* t = ggml_new_tensor_1d(ctx, GGML_TYPE_F32, n); + ggml_set_name(t, "test.weight"); + for (int i = 0; i < n; i++) + ((float*)t->data)[i] = (float)i; + + gguf_context* g = gguf_init_empty(); + gguf_set_val_str(g, "general.architecture", "test"); + gguf_add_tensor(g, t); + REQUIRE(gguf_write_to_file(g, path.c_str(), /*only_meta=*/false)); + gguf_free(g); + ggml_free(ctx); +} + +// Portable env helpers (Windows has no POSIX setenv/unsetenv). +void test_setenv(const char* k, const char* v) { +#if defined(_WIN32) + _putenv_s(k, v); +#else + ::setenv(k, v, 1); +#endif +} +void test_unsetenv(const char* k) { +#if defined(_WIN32) + _putenv_s(k, ""); +#else + ::unsetenv(k); +#endif +} + +// RAII for CRISPASR_GGUF_MMAP, which load_weights reads on every call. Set +// rather than assumed: an inherited `CRISPASR_GGUF_MMAP=0` would otherwise +// silently send both halves of the loop below down the same path. +struct MmapEnv { + std::string saved; + bool had = false; + explicit MmapEnv(const char* value) { + if (const char* v = std::getenv("CRISPASR_GGUF_MMAP")) { + saved = v; + had = true; + } + test_setenv("CRISPASR_GGUF_MMAP", value); + } + ~MmapEnv() { + if (had) + test_setenv("CRISPASR_GGUF_MMAP", saved.c_str()); + else + test_unsetenv("CRISPASR_GGUF_MMAP"); + } + MmapEnv(const MmapEnv&) = delete; + MmapEnv& operator=(const MmapEnv&) = delete; +}; + +} // namespace + +TEST_CASE("release_weight_buffer releases a loaded weight buffer and nulls the handle", "[unit][gguf-release]") { + ggml_backend_t backend = ggml_backend_cpu_init(); + REQUIRE(backend != nullptr); + + const std::string path = "crispasr_test_gguf_release.gguf"; + write_gguf(path, 4096); + + // Both loader paths reach the same release call, and they differ in + // exactly the way that matters: `=1` takes the mmap path, `=0` the legacy + // alloc+copy path that registers no mapping. A release that only worked + // when an entry existed would pass one and fail the other. + for (const char* mmap_mode : {"1", "0"}) { + MmapEnv env(mmap_mode); + INFO("CRISPASR_GGUF_MMAP=" << mmap_mode); + + core_gguf::WeightLoad wl; + REQUIRE(core_gguf::load_weights(path.c_str(), backend, "test-release", wl)); + REQUIRE(wl.buf != nullptr); + // The weights are readable before the release — otherwise "released" + // would be indistinguishable from "never loaded". + ggml_tensor* t = core_gguf::require(wl.tensors, "test.weight", "test-release"); + REQUIRE(t != nullptr); + float first = 0.0f; + ggml_backend_tensor_get(t, &first, 0, sizeof(float)); + REQUIRE(first == 0.0f); + + core_gguf::release_weight_buffer(wl.buf); + REQUIRE(wl.buf == nullptr); + + // A second call through the same handle must not double-free. This is + // the double-release case a caller can actually write; releasing a + // saved copy of the raw pointer is not tested because reading a freed + // ggml_backend_buffer is undefined behaviour whatever the side map does. + core_gguf::release_weight_buffer(wl.buf); + REQUIRE(wl.buf == nullptr); + + ggml_free(wl.ctx); + wl.ctx = nullptr; + wl.tensors.clear(); + } + + std::remove(path.c_str()); + ggml_backend_free(backend); +} + +TEST_CASE("release_weight_buffer accepts a null handle", "[unit][gguf-release]") { + ggml_backend_buffer_t buf = nullptr; + core_gguf::release_weight_buffer(buf); + REQUIRE(buf == nullptr); +} + +TEST_CASE("free_weights releases every buffer it owns", "[unit][gguf-release]") { + ggml_backend_t backend = ggml_backend_cpu_init(); + REQUIRE(backend != nullptr); + + const std::string path = "crispasr_test_gguf_release_free.gguf"; + write_gguf(path, 4096); + + MmapEnv env("1"); + core_gguf::WeightLoad wl; + REQUIRE(core_gguf::load_weights(path.c_str(), backend, "test-free", wl)); + REQUIRE(wl.buf != nullptr); + REQUIRE(wl.ctx != nullptr); + REQUIRE(wl.tensors.count("test.weight") == 1); + + core_gguf::free_weights(wl); + REQUIRE(wl.buf == nullptr); + REQUIRE(wl.buf_cpu == nullptr); + REQUIRE(wl.split_bufs.empty()); + REQUIRE(wl.ctx == nullptr); + REQUIRE(wl.tensors.empty()); + + std::remove(path.c_str()); + ggml_backend_free(backend); +} diff --git a/tests/test-gguf-split-alloc.cpp b/tests/test-gguf-split-alloc.cpp index fd23bdfa0..bb581d419 100644 --- a/tests/test-gguf-split-alloc.cpp +++ b/tests/test-gguf-split-alloc.cpp @@ -5,11 +5,11 @@ // 1. load_weights_split partitions tensors by the is_gpu predicate correctly. // 2. Both GPU and CPU partitions are independently addressable. // 3. Tensor data is correctly loaded into the right partition. -// 4. free_weights cleans up split_bufs without leaks. -// 5. The split_bufs field holds overflow buffers when the GPU partition -// exceeds the 1.5 GiB chunk limit (structural — we verify the field -// exists and is managed; actual multi-GiB allocation is only testable -// on real Vulkan hardware). +// 4. free_weights leaves no buffer handle behind and is idempotent. +// 5. A partition above the chunk limit really does overflow into split_bufs, +// and releasing the partition releases those chunks with it. +// CRISPASR_GGUF_MAX_ALLOC_CHUNK lowers the limit so this needs no +// multi-gigabyte allocation and no Vulkan hardware. #include @@ -21,6 +21,7 @@ #include "gguf.h" #include +#include #include #include #include @@ -54,6 +55,22 @@ void write_multi_tensor_gguf(const std::string& path, int n_tensors, int elems_p ggml_free(ctx); } +// Portable env helpers (Windows has no POSIX setenv/unsetenv). +void test_setenv(const char* k, const char* v) { +#if defined(_WIN32) + _putenv_s(k, v); +#else + ::setenv(k, v, 1); +#endif +} +void test_unsetenv(const char* k) { +#if defined(_WIN32) + _putenv_s(k, ""); +#else + ::unsetenv(k); +#endif +} + // Predicate: layers 0..threshold-1 go to GPU, rest to CPU. struct SplitCtx { int threshold; @@ -194,26 +211,48 @@ TEST_CASE("load_weights_split rejects null backends/predicate", "[unit][gguf-spl ggml_backend_free(be); } -TEST_CASE("free_weights clears split_bufs", "[unit][gguf-split]") { - // Manually construct a WeightLoad with fake split_bufs entries to verify - // free_weights empties the vector. We allocate real buffers so - // ggml_backend_buffer_free is exercised (no UAF / double-free). - ggml_backend_t be = ggml_backend_cpu_init(); - REQUIRE(be); +TEST_CASE("a chunked partition's overflow buffers are released with it", "[unit][gguf-split]") { + // The overflow chunks are owned by the loader and released with the first + // buffer of their own partition. Reaching that branch normally needs a + // partition above 1.5 GiB, so CRISPASR_GGUF_MAX_ALLOC_CHUNK lowers the + // limit far enough for a synthetic model to chunk. + // + // What this proves: the chunked path runs, the overflow buffers are + // recorded, and releasing the partition once — or twice — neither + // double-frees nor leaves a stale handle. What it cannot prove is that + // the memory came back, because ggml exposes no per-backend allocation + // counter to assert against; that half is the code review's. + test_setenv("CRISPASR_GGUF_MAX_ALLOC_CHUNK", "1024"); + + ggml_backend_t gpu_be = ggml_backend_cpu_init(); + ggml_backend_t cpu_be = ggml_backend_cpu_init(); + REQUIRE(gpu_be); + REQUIRE(cpu_be); + const std::string path = "crispasr_test_split_chunked.gguf"; + // 8 tensors of 1 KiB each: several chunks per partition at a 1 KiB limit. + write_multi_tensor_gguf(path, 8, 256); + + SplitCtx sc{4}; core_gguf::WeightLoad wl; - // Allocate two small buffers and push them into split_bufs. - wl.split_bufs.push_back(ggml_backend_alloc_buffer(be, 256)); - wl.split_bufs.push_back(ggml_backend_alloc_buffer(be, 256)); - REQUIRE(wl.split_bufs.size() == 2); - REQUIRE(wl.split_bufs[0] != nullptr); - REQUIRE(wl.split_bufs[1] != nullptr); + REQUIRE(core_gguf::load_weights_split(path.c_str(), gpu_be, cpu_be, test_is_gpu, &sc, "test-chunked", wl)); + REQUIRE(wl.buf != nullptr); + REQUIRE(wl.buf_cpu != nullptr); + // Positive control: without this the case would pass having never chunked. + REQUIRE_FALSE(wl.split_bufs.empty()); core_gguf::free_weights(wl); - - REQUIRE(wl.split_bufs.empty()); REQUIRE(wl.buf == nullptr); REQUIRE(wl.buf_cpu == nullptr); + REQUIRE(wl.split_bufs.empty()); - ggml_backend_free(be); + // Idempotent: the loader's record was taken and erased, not just read, so + // a second release cannot free the same overflow chunks again. + core_gguf::free_weights(wl); + REQUIRE(wl.split_bufs.empty()); + + test_unsetenv("CRISPASR_GGUF_MAX_ALLOC_CHUNK"); + std::remove(path.c_str()); + ggml_backend_free(gpu_be); + ggml_backend_free(cpu_be); } diff --git a/tests/test-region-probe.h b/tests/test-region-probe.h new file mode 100644 index 000000000..c9b5bb90b --- /dev/null +++ b/tests/test-region-probe.h @@ -0,0 +1,97 @@ +// test-region-probe.h — count this process's mapped regions backed by a file. +// +// Test-only helper. The GGUF loader hands a host mmap to the backend on the +// zero-copy path, and the only exact way to assert that the mapping was +// released is to ask the kernel which regions still name the file. A footprint +// or RSS delta would need a threshold and a settling window and would be the +// flaky case in this suite; a region count needs neither. +// +// The count matters, not just presence: load_weights maps the file once per +// *load*, so repeated loads accumulate separate regions and a release that +// dropped only the most recent one would still pass a presence-only check. +// +// count_regions_backed_by() returns (size_t)-1 where the platform offers no +// region enumeration, which callers treat as "cannot assert here". + +#pragma once + +#include + +#if defined(__APPLE__) +#include +#include +#include +#include +#include +#elif defined(__linux__) +#include +#include +#include +#else +#include +#endif + +namespace test_region { + +inline size_t count_regions_backed_by(const std::string& path) { +#if defined(__APPLE__) + // PROC_PIDREGIONPATHINFO returns the region *and* its backing path in one + // record, so the path is always the one belonging to the region reported. + // proc_regionfilename() alone is not usable here: asked about the base of + // an anonymous region it answers with the file of the next region above, + // which counts an unrelated neighbour as a mapping of the weight file. + size_t n = 0; + const pid_t pid = getpid(); + uint64_t addr = 0; + for (;;) { + struct proc_regionwithpathinfo rpi; + const int got = proc_pidinfo(pid, PROC_PIDREGIONPATHINFO, addr, &rpi, sizeof(rpi)); + if (got != (int)sizeof(rpi)) + break; // no region at or above `addr` + if (rpi.prp_vip.vip_path[0] != '\0' && path == rpi.prp_vip.vip_path) + n++; + const uint64_t next = rpi.prp_prinfo.pri_address + rpi.prp_prinfo.pri_size; + if (next <= addr) + break; // no forward progress; stop rather than spin + addr = next; + } + return n; +#elif defined(__linux__) + size_t n = 0; + std::ifstream maps("/proc/self/maps"); + std::string line; + while (std::getline(maps, line)) { + // The path is the last field and may contain spaces, so take + // everything from the field separator rather than tokenizing on space. + const size_t slash = line.find(" /"); + if (slash == std::string::npos) + continue; + if (line.substr(slash + 1) == path) + n++; + } + return n; +#else + (void)path; + return (size_t)-1; +#endif +} + +// True when this platform can answer the question at all. +inline bool region_probe_available() { + return count_regions_backed_by("/") != (size_t)-1; +} + +// The probe reports absolute paths, so a fixture written under a relative name +// has to be resolved before it can be compared. +inline std::string absolute_path_of(const std::string& path) { +#if defined(_WIN32) + return path; +#else + char resolved[PATH_MAX] = {0}; + if (!realpath(path.c_str(), resolved)) + return path; + return std::string(resolved); +#endif +} + +} // namespace test_region