Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,14 @@ Feel free to contribute fixes — PRs welcome.
`synchronized` guard (or `compareAndSet`) on the Java side so `close()` is idempotent and
the native pointer is nulled before the second caller can reach it.

- **`ServerMetrics.getCumulativeTimings()` truncates cumulative token totals to `int`.** The
cumulative token counters are stored as `long` in the JSON but cast to `int` when
constructing `ServerMetrics`, silently truncating values above `Integer.MAX_VALUE`
(~2.1 billion tokens). Fix: widen the field and constructor parameter to `long`.
- **`ServerMetrics.getCumulativeTimings()` truncation — STALE / REFUTED.** This claim was
verified false: `getCumulativeTimings` reads `n_prompt_tokens_processed_total` /
`n_tokens_predicted_total` via `asLong(0L)` into `long` fields and constructs a `Timings`
with `long promptN` / `predictedN` (`ServerMetrics.java` / `Timings.java`) — no int cast
occurs. The real token-count truncation (now fixed) lived in the protocol shims
(`ResponsesStreamTranslator`, `AnthropicStreamTranslator`, `ResponsesApiSupport`,
`AnthropicApiSupport`, `OllamaApiSupport`, `ChatResponseParser.extractUsageField`), which used
`.asInt(0)`; those now use `.asLong(0L)` so sessions above ~2.1B tokens no longer overflow.

- **Unbounded request-body read → OOM DoS.** The HTTP handler reads the entire request body
into a `String`/`byte[]` before parsing it, with no size cap. A client that streams a
Expand Down
1 change: 0 additions & 1 deletion llama/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ endif()

include(FetchContent)

set(BUILD_SHARED_LIBS ON)
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
set(BUILD_SHARED_LIBS OFF)
# Android NDK only declares posix_spawn_file_actions_t as a type alias but
Expand Down
238 changes: 183 additions & 55 deletions llama/src/main/cpp/jllama.cpp

Large diffs are not rendered by default.

68 changes: 68 additions & 0 deletions llama/src/main/cpp/jni_helpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "nlohmann/json.hpp"

#include <atomic>
#include <condition_variable>
#include <map>
#include <memory>
#include <mutex>
Expand Down Expand Up @@ -73,6 +74,24 @@ struct jllama_context {
// that removes the map entry must not free a reader still in use — the receiver's copy
// keeps it alive until next() returns.
std::map<int, std::shared_ptr<server_response_reader>> readers;

// In-flight JNI call reference count, for safe teardown. close() waits for this to reach 0
// before deleting the context, so a concurrent inference/receive call can never dereference
// a freed jllama_context (fixes the close()-vs-inference use-after-free, H2, and the
// close()-during-streaming hang, M4). acquire_jllama_context_impl increments it; the
// jllama_context_guard (set up by REQUIRE_SERVER_CONTEXT) decrements it on scope exit.
std::atomic<int> users{0};
// Set by delete() (teardown) before terminating the worker, so any in-flight
// reader parked in next()/wait_for_all() observes a stop signal and unwinds
// instead of polling forever (which would otherwise keep `users` > 0 and hang
// close()). The streaming/blocking should_stop lambdas read this.
std::atomic<bool> closing{false};
// Serializes the "last user released" notification (paired with cv). The users
// decrement in release_jllama_context_impl happens UNDER this lock so delete()'s
// cv.wait cannot observe users==0 and free m/cv before the releaser finishes
// touching them (otherwise a use-after-free on the condvar/mutex themselves).
std::mutex m;
std::condition_variable cv;
};

// Removes the streaming reader entry for `id_task` under the readers_mutex.
Expand Down Expand Up @@ -112,6 +131,55 @@ inline void erase_reader(jllama_context *jctx, int id_task) {
return reinterpret_cast<jllama_context *>(handle); // NOLINT(*-no-int-to-ptr)
}

// ---------------------------------------------------------------------------
// acquire / release_jllama_context_impl + jllama_context_guard
//
// Reference-count in-flight JNI calls against the jllama_context so that
// close() can wait for all of them to drain before deleting the context. Every
// entry point that dereferences jctx goes through REQUIRE_SERVER_CONTEXT, which
// constructs a jllama_context_guard; the guard's destructor calls
// release_jllama_context_impl (even on early return), decrementing users.
//
// close() (delete()) first nulls the Java handle under g_ctx_mutex so no NEW
// acquire can succeed, terminates+joins the worker, then waits on jctx->cv for
// users to reach 0 — guaranteeing no call is mid-use when the context is freed
// (fixes H2 use-after-free and M4 close()-during-streaming hang). g_ctx_mutex is
// defined in jllama.cpp and declared extern here.
// ---------------------------------------------------------------------------
extern std::mutex g_ctx_mutex;

[[nodiscard]] inline jllama_context *acquire_jllama_context_impl(JNIEnv *env, jobject obj, jfieldID field_id) {
std::lock_guard<std::mutex> lk(g_ctx_mutex);
const jlong handle = env->GetLongField(obj, field_id);
if (handle == 0) {
return nullptr;
}
auto *jctx = reinterpret_cast<jllama_context *>(handle); // NOLINT(*-no-int-to-ptr)
jctx->users.fetch_add(1, std::memory_order_relaxed);
return jctx;
}

inline void release_jllama_context_impl(jllama_context *jctx) {
if (jctx == nullptr) {
return;
}
// Decrement under jctx->m so delete()'s cv.wait (which holds jctx->m while checking the
// predicate) cannot observe users==0 and free m/cv before this releaser has finished
// notifying — that would be a use-after-free on the condvar/mutex themselves (H2/H3).
std::lock_guard<std::mutex> lk(jctx->m);
// fetch_sub returns the previous value; when it was 1 we just dropped the last reference.
if (jctx->users.fetch_sub(1, std::memory_order_acq_rel) == 1) {
jctx->cv.notify_all();
}
}

// RAII guard: releases the jllama_context user reference at scope exit, including
// on early return. REQUIRE_SERVER_CONTEXT uses this so every entry point is covered.
struct jllama_context_guard {
jllama_context *ptr;
~jllama_context_guard() { release_jllama_context_impl(ptr); }
};

// ---------------------------------------------------------------------------
// require_json_field_impl
//
Expand Down
31 changes: 23 additions & 8 deletions llama/src/main/cpp/json_helpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

#include "nlohmann/json.hpp"

#include <cmath>
#include <optional>
#include <stdexcept>
#include <string>
Expand Down Expand Up @@ -90,7 +91,15 @@
json arr = json::array();
for (const auto &result : results) {
const auto out = result->to_json();
int index = out["index"].get<int>();
// Defensive: a malformed/absent "index" or an out-of-range value would otherwise
// throw json::type_error or index documents[] out of bounds (M2).
if (!out.contains("index")) {
throw std::invalid_argument("rerank result is missing the 'index' field");
}
const int index = out["index"].get<int>();
if (index < 0 || static_cast<size_t>(index) >= documents.size()) {
throw std::invalid_argument("rerank result index " + std::to_string(index) + " out of range");
}
float score = out["score"].get<float>();
arr.push_back({{"document", documents[index]}, {"index", index}, {"score", score}});
}
Expand Down Expand Up @@ -168,11 +177,13 @@
if (!config.contains("slot_prompt_similarity")) {
return std::nullopt;
}
const float v = config["slot_prompt_similarity"].get<float>();
if (v < 0.0f || v > 1.0f) {
// Coerce via double so an integer-valued config (e.g. 0 or 1) is accepted rather than
// throwing json::type_error; the range check below preserves the [0.0, 1.0] contract (L5).
const double v = config["slot_prompt_similarity"].get<double>();
if (v < 0.0 || v > 1.0) {
throw std::invalid_argument("slot_prompt_similarity must be between 0.0 and 1.0");
}
return v;
return static_cast<float>(v);
}

// ---------------------------------------------------------------------------
Expand All @@ -188,11 +199,15 @@
if (!config.contains(key)) {
return std::nullopt;
}
const int v = config[key].get<int>();
if (v <= 0) {
throw std::invalid_argument(std::string(key) + " must be greater than 0");
// Coerce via double so an integer-valued config is accepted rather than throwing
// json::type_error; require a positive integer per the documented contract (L5).
const double raw = config[key].get<double>();
// Reject non-positive, non-integer, or > INT_MAX values: a whole-number JSON value above
// INT_MAX would overflow static_cast<int> (UB).
if (raw <= 0.0 || raw != std::floor(raw) || raw > 2147483647.0) {
throw std::invalid_argument(std::string(key) + " must be a positive integer");
}
return v;
return static_cast<int>(raw);
}

// ---------------------------------------------------------------------------
Expand Down
86 changes: 75 additions & 11 deletions llama/src/main/cpp/native_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ struct native_server {
int exit_code = -1;
};

// Standard-UTF8 extraction used for argv/commands (declared here; defined below).
static std::string jstring_to_utf8(JNIEnv *env, jstring js);

// Copies the forwarded Java argv into srv->args/argv with a synthetic argv[0]. The argv pointers
// reference the std::string storage in `args`, which is filled once (with reserve) and never
// mutated afterwards, so the pointers stay valid for the worker's lifetime.
Expand All @@ -56,11 +59,7 @@ void fill_native_server_args(JNIEnv *env, jobjectArray jargs, native_server *srv
for (jsize i = 0; i < n; ++i) {
auto js = static_cast<jstring>(env->GetObjectArrayElement(jargs, i));
if (js != nullptr) {
const char *chars = env->GetStringUTFChars(js, nullptr);
srv->args.emplace_back(chars != nullptr ? chars : "");
if (chars != nullptr) {
env->ReleaseStringUTFChars(js, chars);
}
srv->args.emplace_back(jstring_to_utf8(env, js));
env->DeleteLocalRef(js);
} else {
srv->args.emplace_back("");
Expand All @@ -82,6 +81,62 @@ void throw_llama_exception(JNIEnv *env, const char *message) {
}
}

// Standard-UTF8 extraction (mirrors jllama.cpp parse_jstring): GetStringUTFChars yields
// Modified-UTF8 (CESU-8), which mangles non-BMP chars (emoji, CJK extensions) in argv such as
// model paths. We go through String.getBytes("UTF-8") instead (H1).
std::string jstring_to_utf8(JNIEnv *env, jstring js) {
if (js == nullptr) {
return "";
}
// Resolve and cache the JNI class/method/field IDs once (server-startup is the only caller,
// but it can run many times — e.g. once per forwarded argv element). FindClass returns a
// frame-local ref, so promote the reused handles to global refs for caching (MED #6).
static jclass str_cls = nullptr;
static jmethodID get_bytes = nullptr;
static jobject utf8 = nullptr;
static std::once_flag init_flag;
std::call_once(init_flag, [&]() {
jclass local = env->FindClass("java/lang/String");
if (local == nullptr) {
return;
}
jmethodID gb = env->GetMethodID(local, "getBytes", "(Ljava/nio/charset/Charset;)[B");
jclass charset_cls = env->FindClass("java/nio/charset/StandardCharsets");
jfieldID utf8_f =
charset_cls ? env->GetStaticFieldID(charset_cls, "UTF_8", "Ljava/nio/charset/Charset;") : nullptr;
if (gb == nullptr || charset_cls == nullptr || utf8_f == nullptr) {
if (charset_cls != nullptr) {
env->DeleteLocalRef(charset_cls);
}
env->DeleteLocalRef(local);
return;
}
jobject u = env->GetStaticObjectField(charset_cls, utf8_f);
env->DeleteLocalRef(charset_cls);
str_cls = static_cast<jclass>(env->NewGlobalRef(local));
get_bytes = gb;
utf8 = env->NewGlobalRef(u);
env->DeleteLocalRef(local);
env->DeleteLocalRef(u);
});
if (str_cls == nullptr || get_bytes == nullptr || utf8 == nullptr) {
return "";
}

jbyteArray bytes = (jbyteArray)env->CallObjectMethod(js, get_bytes, utf8);
std::string out;
if (bytes != nullptr) {
const jsize blen = env->GetArrayLength(bytes);
jbyte *elems = env->GetByteArrayElements(bytes, nullptr);
if (elems != nullptr) {
out.assign(reinterpret_cast<char *>(elems), static_cast<size_t>(blen));
env->ReleaseByteArrayElements(bytes, elems, JNI_ABORT);
}
env->DeleteLocalRef(bytes);
}
return out;
}

} // namespace

extern "C" {
Expand Down Expand Up @@ -151,6 +206,13 @@ JNIEXPORT jlong JNICALL Java_net_ladenthin_llama_server_NativeServer_startAttach
}
auto *jctx = reinterpret_cast<jllama_context *>(model_handle); // NOLINT(*-no-int-to-ptr)

// Contract (M6): llama_server_attach drives the HTTP frontend on a NEW worker thread but
// must NOT start a second task-processing loop on the shared server_context — the model's
// own worker (spawned in load_model_impl) already runs start_loop(). Likewise,
// stopNativeServer's llama_server_request_shutdown() must terminate only the HTTP frontend,
// not the attached model's worker, so the LlamaModel remains usable after the server closes
// (documented order: close server first, then model). If a future upstream change breaks
// either assumption, this attach path needs revisiting.
auto *srv = new native_server();
fill_native_server_args(env, jargs, srv);

Expand All @@ -159,9 +221,15 @@ JNIEXPORT jlong JNICALL Java_net_ladenthin_llama_server_NativeServer_startAttach
llama_server_set_embedded(true);

server_context *ctx_server = &jctx->server;
srv->worker = std::thread([srv, ctx_server]() {
// Take a jllama_context user reference for the lifetime of the attach worker so that
// LlamaModel.close() cannot free jctx (and thus ctx_server) out from under the running
// HTTP frontend (use-after-free, MED #2). delete() in jllama.cpp waits on the user-count
// condition variable, so close() blocks until this thread exits.
jctx->users.fetch_add(1, std::memory_order_relaxed);
srv->worker = std::thread([srv, ctx_server, jctx]() {
srv->exit_code = llama_server_attach(static_cast<int>(srv->argv.size()), srv->argv.data(), *ctx_server);
srv->finished.store(true);
release_jllama_context_impl(jctx);
});

return reinterpret_cast<jlong>(srv);
Expand All @@ -173,11 +241,7 @@ JNIEXPORT void JNICALL Java_net_ladenthin_llama_server_NativeServer_setWorkerCom
// model manager (server-models.cpp, patches/0008) reads when spawning worker instances.
std::string value;
if (jcommand != nullptr) {
const char *chars = env->GetStringUTFChars(jcommand, nullptr);
if (chars != nullptr) {
value = chars;
env->ReleaseStringUTFChars(jcommand, chars);
}
value = jstring_to_utf8(env, jcommand);
}
#if defined(_WIN32)
_putenv_s("LLAMA_SERVER_WORKER_CMD", value.c_str()); // empty value removes the variable
Expand Down
16 changes: 10 additions & 6 deletions llama/src/main/cpp/tts_engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

#include <algorithm>
#include <cstdint>
#include <mutex>
#include <regex>
#include <string>
#include <vector>
Expand All @@ -44,6 +45,9 @@ struct tts_engine {
const llama_vocab *vocab = nullptr;
outetts_version tts_version = OUTETTS_V0_2;
int n_threads = 4;
// Serializes engine_synthesize: it drives llama_decode/llama_encode on the shared
// ctx_ttc/ctx_cts contexts, so two threads on one engine would race (M5).
std::mutex synthesize_mutex;
};

tts_engine *engine_init(const std::string &ttc_model_path, const std::string &cts_model_path, int n_gpu_layers,
Expand Down Expand Up @@ -98,6 +102,8 @@ bool engine_synthesize(tts_engine *engine, const std::string &text, int n_predic
err = "engine is null";
return false;
}
// Serialize against concurrent calls on the same engine (M5).
std::lock_guard<std::mutex> engine_lock(engine->synthesize_mutex);
const llama_vocab *vocab = engine->vocab;

common_params_sampling sparams;
Expand Down Expand Up @@ -185,12 +191,10 @@ bool engine_synthesize(tts_engine *engine, const std::string &text, int n_predic
llama_batch_free(batch);
common_sampler_free(smpl);

// Keep only audio codec tokens (151672..155772) and rebase to codec ids.
codes.erase(std::remove_if(codes.begin(), codes.end(), [](llama_token t) { return t < 151672 || t > 155772; }),
codes.end());
for (auto &token : codes) {
token -= 151672;
}
// Keep only audio codec tokens and rebase to 0-based codec ids. The window is identical
// for OuteTTS V0_2 and V0_3 (verified against upstream tools/tts/tts.cpp, see M3 in
// docs/plan-bugs-and-issues.md) — delegated to the pure filter_ouetts_codec_tokens helper.
filter_ouetts_codec_tokens(codes);
if (codes.empty()) {
err = "no audio codes were generated";
return false;
Expand Down
21 changes: 21 additions & 0 deletions llama/src/main/cpp/tts_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#ifndef JLLAMA_TTS_ENGINE_H
#define JLLAMA_TTS_ENGINE_H

#include <algorithm>
#include <cstdint>
#include <string>
#include <vector>
Expand All @@ -31,6 +32,26 @@ bool engine_synthesize(tts_engine *engine, const std::string &text, int n_predic
// Release both models / contexts. Safe on nullptr.
void engine_free(tts_engine *engine);

// ---------------------------------------------------------------------------
// OuteTTS audio-codec token filtering (pure transform, testable without a model).
//
// The TTC model emits codec tokens in a high id window; the CTS vocoder only
// understands the 0-based codec ids inside [k_ouetts_codec_lo, k_ouetts_codec_hi].
// This window is identical for OuteTTS V0_2 and V0_3 (verified against upstream
// tools/tts/tts.cpp — see M3 in docs/plan-bugs-and-issues.md), so it is NOT gated
// on tts_version. Keep only tokens in the window and rebase them to 0-based ids.
constexpr int32_t k_ouetts_codec_lo = 151672;
constexpr int32_t k_ouetts_codec_hi = 155772;

inline void filter_ouetts_codec_tokens(std::vector<int32_t> &tokens) {
tokens.erase(std::remove_if(tokens.begin(), tokens.end(),
[](int32_t t) { return t < k_ouetts_codec_lo || t > k_ouetts_codec_hi; }),
tokens.end());
for (auto &t : tokens) {
t -= k_ouetts_codec_lo;
}
}

} // namespace jllama_tts

#endif // JLLAMA_TTS_ENGINE_H
Loading
Loading