Repository navigation
Conversation
… `Llama` Creating a new `Llama` instance released the previous logger's thread-safe function before installing the new one. The old function's deferred finalizer then cleared the global logger callback flag, so native llama.cpp log lines stopped reaching the custom logger of the new instance and were printed to the process stdout instead. Install the new logger before releasing the previous one, and only clear the callback flag when the finalized logger is still the current one. Closes: withcatai#684
This branch has not been deployed
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.
Summary
Fixes #684.
Each
new Llama()calls the addon'ssetLogger, which released the previous logger's thread-safe function before installing the new one. The old function's deferred finalizer then ran and cleared the global "JS logger callback set" flag, so from the secondgetLlama()call on, native llama.cpp log lines no longer reached any customloggerand were printed by the addon's fallback to stdout/stderr instead.Changes
llama/addon/globals/addonLog.cpp: track the currently installed logger's context, install the new thread-safe function before releasing the previous one, and only clear the global callback flag when the finalized logger is still the current one. Single-instance behavior is unchanged.test/modelDependent/llama/logger.test.ts: new regression test that loads a model through two sequentialgetLlama()instances with separate loggers and asserts both receive native llama.cpp log lines (llama_model_loader). Without the fix, the second logger only receives the few TypeScript-side lines, so the test fails.Verification
llama_model_loaderlines leak to process stdout - matching the issue report.vitest run ./test/modelDependent/llama/logger.test.ts- passes (CPU build,NODE_LLAMA_CPP_GPU=false,bge-small-en-v1.5-q8_0.gguf).tsc --noEmit --project tsconfig.json- cleaneslinton the changed files - cleanTested on Linux x64 only; the change is platform-independent N-API ordering logic.