Skip to content

Commit ea28c80

Browse files
committed
fix bugs
1 parent 62e17b2 commit ea28c80

10 files changed

Lines changed: 313 additions & 128 deletions

File tree

crates/vespera/src/multipart.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,11 +498,16 @@ fn tiny_scalar_limit(limit_bytes: Option<usize>) -> usize {
498498

499499
/// Parse a string as a boolean using clap-style conventions.
500500
///
501+
/// Surrounding ASCII whitespace is ignored, so a multipart text value that
502+
/// arrives with incidental padding (e.g. a trailing newline) parses like the
503+
/// trimmed token — matching the numeric field impls, which `text.trim().parse()`.
504+
///
501505
/// Accepted truthy values: `true`, `yes`, `y`, `1`, `on`
502506
/// Accepted falsy values: `false`, `no`, `n`, `0`, `off`
503507
fn str_to_bool(s: &str) -> Option<bool> {
504508
const TRUTHY: [&str; 5] = ["true", "yes", "y", "1", "on"];
505509
const FALSY: [&str; 5] = ["false", "no", "n", "0", "off"];
510+
let s = s.trim();
506511
if TRUTHY.iter().any(|t| s.eq_ignore_ascii_case(t)) {
507512
Some(true)
508513
} else if FALSY.iter().any(|f| s.eq_ignore_ascii_case(f)) {

crates/vespera/src/multipart/tests.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,23 @@ fn test_str_to_bool_invalid() {
2727
}
2828
}
2929

30+
#[test]
31+
fn test_str_to_bool_trims_surrounding_whitespace() {
32+
// Multipart text values can arrive with incidental surrounding whitespace
33+
// (e.g. a trailing newline from a client); bool must tolerate it exactly as
34+
// the numeric field impls do (`text.trim().parse()`), so a padded token
35+
// parses like the bare token instead of being rejected.
36+
assert_eq!(str_to_bool(" true "), Some(true));
37+
assert_eq!(str_to_bool("true\n"), Some(true));
38+
assert_eq!(str_to_bool("\tyes\r\n"), Some(true));
39+
assert_eq!(str_to_bool(" false"), Some(false));
40+
assert_eq!(str_to_bool("off\n"), Some(false));
41+
// Trim only touches the ends — internal whitespace stays invalid, and a
42+
// whitespace-only value is still `None`.
43+
assert_eq!(str_to_bool("tr ue"), None);
44+
assert_eq!(str_to_bool(" "), None);
45+
}
46+
3047
// ─── Display tests for all error variants ───────────────────────────
3148

3249
#[test]

crates/vespera_jni/src/jni_buf.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ pub fn read_byte_array_region(
6363
// path) or the exact positive `InputStream.read(byte[])` count after
6464
// checking it does not exceed the fixed streaming buffer length.
6565
// * The destination is `vec`'s reserved-but-uninitialised capacity
66-
// (`with_capacity(len)` reserved exactly `len` bytes). Only a raw
66+
// (`try_reserve_exact(len)` reserved exactly `len` bytes). Only a raw
6767
// `*mut jbyte` is passed to JNI — no `&mut [i8]` over uninitialised
6868
// memory is created. `u8` and `jbyte` (`i8`) share size/alignment.
6969
unsafe {

crates/vespera_jni/src/jni_impl.rs

Lines changed: 8 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use futures_util::FutureExt;
1010
use jni::EnvUnowned;
1111
use jni::errors::ThrowRuntimeExAndDefault;
1212
use jni::objects::{JByteArray, JClass, JObject};
13-
use jni::sys::{jbyteArray, jint};
13+
use jni::sys::jbyteArray;
1414

1515
use crate::daemon_env::with_cached_daemon_env;
1616
use crate::streaming_closures::{
@@ -24,6 +24,13 @@ use crate::streaming_closures::{
2424
mod streaming_buffer;
2525
use streaming_buffer::{PullPushBuffers, mark_streaming_buffer_reusable};
2626

27+
// Runtime / streaming configuration JNI hooks (seeded from
28+
// `VesperaBridge.init()` before the first dispatch) live in a sidecar
29+
// module so this file stays focused on the per-request dispatch symbols.
30+
#[path = "jni_impl_config.rs"]
31+
mod config;
32+
pub use config::{runtime_worker_threads, streaming_chunk_size};
33+
2734
/// Multi-threaded Tokio runtime shared across all JNI calls.
2835
///
2936
/// Worker thread count defaults to Tokio's heuristic (number of
@@ -41,11 +48,6 @@ pub static RUNTIME: LazyLock<tokio::runtime::Runtime> = LazyLock::new(|| {
4148
.expect("failed to create Tokio runtime")
4249
});
4350

44-
const MIN_RUNTIME_WORKERS: usize = 1;
45-
const MAX_RUNTIME_WORKERS: usize = 1024;
46-
47-
static RUNTIME_WORKER_THREADS: std::sync::OnceLock<Option<usize>> = std::sync::OnceLock::new();
48-
4951
/// Cap on each per-thread sync runtime's blocking pool.
5052
///
5153
/// [`block_on_sync_runtime`] builds ONE current-thread runtime per calling
@@ -200,106 +202,6 @@ use support::{
200202
setup_stream_with_header, throw_streaming_abort,
201203
};
202204

203-
/// Worker thread count for the shared [`RUNTIME`], resolved once
204-
/// (first hit wins, then fixed for the process lifetime):
205-
///
206-
/// 1. [`set_runtime_worker_threads`] called before the runtime is
207-
/// first used (the `configureRuntime0` JNI hook from
208-
/// `VesperaBridge.init()` lands here)
209-
/// 2. `VESPERA_RUNTIME_WORKERS` environment variable
210-
/// 3. `None` — Tokio's default (number of logical CPUs)
211-
///
212-
/// Values are clamped to `[1, 1024]`.
213-
#[must_use]
214-
pub fn runtime_worker_threads() -> Option<usize> {
215-
*RUNTIME_WORKER_THREADS.get_or_init(|| {
216-
std::env::var("VESPERA_RUNTIME_WORKERS")
217-
.ok()
218-
.and_then(|raw| raw.trim().parse::<usize>().ok())
219-
.map(|v| v.clamp(MIN_RUNTIME_WORKERS, MAX_RUNTIME_WORKERS))
220-
})
221-
}
222-
223-
/// Override the shared runtime's worker thread count **before the
224-
/// first dispatch**. Returns `false` when the value was already
225-
/// fixed. Clamped to `[1, 1024]`.
226-
pub fn set_runtime_worker_threads(workers: usize) -> bool {
227-
RUNTIME_WORKER_THREADS
228-
.set(Some(
229-
workers.clamp(MIN_RUNTIME_WORKERS, MAX_RUNTIME_WORKERS),
230-
))
231-
.is_ok()
232-
}
233-
234-
/// `com.devfive.vespera.bridge.VesperaBridge.configureRuntime0(int) -> void`
235-
///
236-
/// Seeds the shared Tokio runtime's worker thread count **before
237-
/// the first dispatch**. Values `<= 0` leave the setting
238-
/// untouched (env var / Tokio default applies). Calls after the
239-
/// configuration is fixed are silently ignored.
240-
#[unsafe(no_mangle)]
241-
pub extern "system" fn Java_com_devfive_vespera_bridge_VesperaBridge_configureRuntime0<'local>(
242-
_unowned_env: EnvUnowned<'local>,
243-
_class: JClass<'local>,
244-
worker_threads: jint,
245-
) {
246-
// Defensive `catch_unwind`: this body cannot panic today, but it is
247-
// an `extern "system"` JNI symbol, so guard it for consistency with
248-
// the dispatch symbols — an unwind must never cross the FFI boundary.
249-
let _ = std::panic::catch_unwind(|| {
250-
if let Ok(workers) = usize::try_from(worker_threads)
251-
&& workers > 0
252-
{
253-
let _ = set_runtime_worker_threads(workers);
254-
}
255-
});
256-
}
257-
258-
/// Per-chunk buffer size for streaming dispatches.
259-
///
260-
/// Resolved once per process by
261-
/// [`vespera_inprocess::streaming_chunk_bytes`] (default 256 KiB;
262-
/// override via the `VESPERA_STREAMING_CHUNK_BYTES` env var or the
263-
/// `configureStreaming0` JNI setter called from
264-
/// `VesperaBridge.init()`). Large enough to amortise JNI call
265-
/// overhead, small enough to keep memory bounded for multi-GB
266-
/// streams. Subsequent calls are a single atomic load.
267-
pub fn streaming_chunk_size() -> usize {
268-
vespera_inprocess::streaming_chunk_bytes()
269-
}
270-
271-
/// `com.devfive.vespera.bridge.VesperaBridge.configureStreaming0(int, int) -> void`
272-
///
273-
/// Seeds the process-wide streaming configuration **before the
274-
/// first dispatch**. Values `<= 0` leave the corresponding
275-
/// setting untouched (env var / default applies). Calls after
276-
/// the configuration is fixed (first dispatch already ran, or a
277-
/// previous call set it) are silently ignored — the JNI side has
278-
/// no use for the failure signal beyond logging, which Java owns.
279-
#[unsafe(no_mangle)]
280-
pub extern "system" fn Java_com_devfive_vespera_bridge_VesperaBridge_configureStreaming0<'local>(
281-
_unowned_env: EnvUnowned<'local>,
282-
_class: JClass<'local>,
283-
chunk_bytes: jint,
284-
channel_capacity: jint,
285-
) {
286-
// Defensive `catch_unwind` — see `configureRuntime0`: keep every JNI
287-
// `extern "system"` symbol panic-safe even though this body cannot
288-
// panic with the current setters.
289-
let _ = std::panic::catch_unwind(|| {
290-
if let Ok(bytes) = usize::try_from(chunk_bytes)
291-
&& bytes > 0
292-
{
293-
let _ = vespera_inprocess::set_streaming_chunk_bytes(bytes);
294-
}
295-
if let Ok(slots) = usize::try_from(channel_capacity)
296-
&& slots > 0
297-
{
298-
let _ = vespera_inprocess::set_streaming_channel_capacity(slots);
299-
}
300-
});
301-
}
302-
303205
/// `com.devfive.vespera.bridge.VesperaBridge.dispatchBytes(byte[]) -> byte[]`
304206
///
305207
/// **Synchronous** binary wire-format JNI entry point. Blocks the
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
//! Runtime / streaming configuration JNI hooks.
2+
//!
3+
//! These symbols are seeded from `VesperaBridge.init()` **before the first
4+
//! dispatch** and then fixed for the process lifetime. They are split out of
5+
//! `jni_impl.rs` (which owns the per-request dispatch symbols) so each file
6+
//! keeps a single concern — and stays within the 1000-line source cap.
7+
8+
use jni::EnvUnowned;
9+
use jni::objects::JClass;
10+
use jni::sys::jint;
11+
12+
const MIN_RUNTIME_WORKERS: usize = 1;
13+
const MAX_RUNTIME_WORKERS: usize = 1024;
14+
15+
static RUNTIME_WORKER_THREADS: std::sync::OnceLock<Option<usize>> = std::sync::OnceLock::new();
16+
17+
/// Worker thread count for the shared [`RUNTIME`](super::RUNTIME), resolved once
18+
/// (first hit wins, then fixed for the process lifetime):
19+
///
20+
/// 1. [`set_runtime_worker_threads`] called before the runtime is
21+
/// first used (the `configureRuntime0` JNI hook from
22+
/// `VesperaBridge.init()` lands here)
23+
/// 2. `VESPERA_RUNTIME_WORKERS` environment variable
24+
/// 3. `None` — Tokio's default (number of logical CPUs)
25+
///
26+
/// Values are clamped to `[1, 1024]`.
27+
#[must_use]
28+
pub fn runtime_worker_threads() -> Option<usize> {
29+
*RUNTIME_WORKER_THREADS.get_or_init(|| {
30+
std::env::var("VESPERA_RUNTIME_WORKERS")
31+
.ok()
32+
.and_then(|raw| raw.trim().parse::<usize>().ok())
33+
.map(|v| v.clamp(MIN_RUNTIME_WORKERS, MAX_RUNTIME_WORKERS))
34+
})
35+
}
36+
37+
/// Override the shared runtime's worker thread count **before the
38+
/// first dispatch**. Returns `false` when the value was already
39+
/// fixed. Clamped to `[1, 1024]`.
40+
pub fn set_runtime_worker_threads(workers: usize) -> bool {
41+
RUNTIME_WORKER_THREADS
42+
.set(Some(
43+
workers.clamp(MIN_RUNTIME_WORKERS, MAX_RUNTIME_WORKERS),
44+
))
45+
.is_ok()
46+
}
47+
48+
/// `com.devfive.vespera.bridge.VesperaBridge.configureRuntime0(int) -> void`
49+
///
50+
/// Seeds the shared Tokio runtime's worker thread count **before
51+
/// the first dispatch**. Values `<= 0` leave the setting
52+
/// untouched (env var / Tokio default applies). Calls after the
53+
/// configuration is fixed are silently ignored.
54+
#[unsafe(no_mangle)]
55+
pub extern "system" fn Java_com_devfive_vespera_bridge_VesperaBridge_configureRuntime0<'local>(
56+
_unowned_env: EnvUnowned<'local>,
57+
_class: JClass<'local>,
58+
worker_threads: jint,
59+
) {
60+
// Defensive `catch_unwind`: this body cannot panic today, but it is
61+
// an `extern "system"` JNI symbol, so guard it for consistency with
62+
// the dispatch symbols — an unwind must never cross the FFI boundary.
63+
let _ = std::panic::catch_unwind(|| {
64+
if let Ok(workers) = usize::try_from(worker_threads)
65+
&& workers > 0
66+
{
67+
let _ = set_runtime_worker_threads(workers);
68+
}
69+
});
70+
}
71+
72+
/// Per-chunk buffer size for streaming dispatches.
73+
///
74+
/// Resolved once per process by
75+
/// [`vespera_inprocess::streaming_chunk_bytes`] (default 256 KiB;
76+
/// override via the `VESPERA_STREAMING_CHUNK_BYTES` env var or the
77+
/// `configureStreaming0` JNI setter called from
78+
/// `VesperaBridge.init()`). Large enough to amortise JNI call
79+
/// overhead, small enough to keep memory bounded for multi-GB
80+
/// streams. Subsequent calls are a single atomic load.
81+
pub fn streaming_chunk_size() -> usize {
82+
vespera_inprocess::streaming_chunk_bytes()
83+
}
84+
85+
/// `com.devfive.vespera.bridge.VesperaBridge.configureStreaming0(int, int) -> void`
86+
///
87+
/// Seeds the process-wide streaming configuration **before the
88+
/// first dispatch**. Values `<= 0` leave the corresponding
89+
/// setting untouched (env var / default applies). Calls after
90+
/// the configuration is fixed (first dispatch already ran, or a
91+
/// previous call set it) are silently ignored — the JNI side has
92+
/// no use for the failure signal beyond logging, which Java owns.
93+
#[unsafe(no_mangle)]
94+
pub extern "system" fn Java_com_devfive_vespera_bridge_VesperaBridge_configureStreaming0<'local>(
95+
_unowned_env: EnvUnowned<'local>,
96+
_class: JClass<'local>,
97+
chunk_bytes: jint,
98+
channel_capacity: jint,
99+
) {
100+
// Defensive `catch_unwind` — see `configureRuntime0`: keep every JNI
101+
// `extern "system"` symbol panic-safe even though this body cannot
102+
// panic with the current setters.
103+
let _ = std::panic::catch_unwind(|| {
104+
if let Ok(bytes) = usize::try_from(chunk_bytes)
105+
&& bytes > 0
106+
{
107+
let _ = vespera_inprocess::set_streaming_chunk_bytes(bytes);
108+
}
109+
if let Ok(slots) = usize::try_from(channel_capacity)
110+
&& slots > 0
111+
{
112+
let _ = vespera_inprocess::set_streaming_channel_capacity(slots);
113+
}
114+
});
115+
}

crates/vespera_jni/src/jni_impl_runtime_config_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use super::{runtime_worker_threads, set_runtime_worker_threads};
1+
use super::config::{runtime_worker_threads, set_runtime_worker_threads};
22

33
/// One test owns the process-global `OnceLock`: setter wins,
44
/// clamping applies, and later writes are rejected.

0 commit comments

Comments
 (0)