sampling: catch C++ exceptions from sample/apply at the FFI boundary - #1093
Open
GuiBarradas wants to merge 1 commit into
Open
sampling: catch C++ exceptions from sample/apply at the FFI boundary#1093GuiBarradas wants to merge 1 commit into
GuiBarradas wants to merge 1 commit into
Conversation
llama.cpp can throw during sampling (for example when a grammar masks every remaining candidate, which happens when a grammar sampler runs after a truncation sampler such as top-k). The exception unwinds across the FFI boundary, which Rust cannot catch, and the process aborts with "fatal runtime error: Rust cannot catch foreign exceptions". PR utilityai#874 added try/catch wrappers for grammar construction and accept. This extends the same pattern to the sampling path: wrap llama_sampler_sample and llama_sampler_apply in the C++ shim (llama_rs_sampler_sample / llama_rs_sampler_apply) and expose LlamaSampler::try_sample and LlamaTokenDataArray::try_apply_sampler, which return SamplerSampleError instead of aborting. Closes utilityai#1082
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1082
llama.cpp can throw a C++ exception during sampling, for example when a grammar masks every remaining candidate (a grammar sampler placed after a truncation sampler such as top-k). The exception unwinds across the FFI boundary, Rust cannot catch foreign exceptions, and the process aborts:
#874 added try/catch wrappers for grammar construction and
accept. This extends the same pattern to the sampling path:wrapper_common.cpp/.h):llama_rs_sampler_sampleandllama_rs_sampler_apply, wrappingllama_sampler_sample/llama_sampler_applyin try/catch and returningllama_rs_status, mirroring the existingllama_rs_sampler_accept.LlamaSampler::try_sampleandLlamaTokenDataArray::try_apply_sampler, returning a newSamplerSampleErrorinstead of aborting. Gated on thecommonfeature, liketry_accept. The existingsample/applyare unchanged.Verified on Windows (CPU build): the crate compiles, and a chain of
[top_k(1), grammar("root ::= \"unlikely\""), dist]that aborted the process viasamplenow returnsErr(SamplerSampleError::FfiError(-3))viatry_sample, with the process surviving. This is the same failure real callers hit (nobodywho-ooo/nobodywho#421, whose generation loop usessample).No tests added, since the failure needs a loaded model and the crate's unit tests do not use one. Happy to add one under a feature/env gate if you prefer.