allthingssecurity opened a new pull request, #27593:
URL: https://github.com/apache/camel/pull/27593

   # Description
   
   [CAMEL-25456](https://issues.apache.org/jira/browse/CAMEL-25456)
   
   A `wasm` producer and a `wasm` expression each create one Endive `Instance` 
of the module and use it for every message (one at a time, under the lock of 
`WasmFunction`). When the guest function traps (a Rust `panic!`/`unwrap()` on 
bad input, any Wasm trap) or the calling thread is interrupted, 
`WasmFunction.run()` kept using that instance:
   
   1. **Memory leak.** Whatever the guest allocated before the trap stays 
allocated in the instance, and nothing resets it. With the module shipped for 
the tests (`functions.wasm`, `process`), 32 messages with a 256 KiB body that 
is not UTF-8 grew the linear memory from 17 to 153 pages (+8.5 MiB); a guest 
that allocates 1 MiB and traps grew it from 17 to 1055 pages in 64 calls. Rust 
and Go modules declare no memory maximum (65536 pages, 4 GiB), so a stream of 
bad messages grows one endpoint's heap until the JVM runs out.
   2. **Wrong size on the error path.** When the function sets the error bit on 
the returned size, the error message was read with the size without the bit, 
but the buffer was released with `dealloc(ptr, size | 1 << 31)` (the guest 
received -2147483644 instead of 4). With the `dealloc` shown in the 
documentation (`Vec::from_raw_parts(ptr, 0, len)`) that is a capacity above 
`isize::MAX`, undefined behaviour in Rust.
   3. **Original exception lost.** After a failure the `finally` block called 
`dealloc` on the failed instance. On an interrupted thread that call throws 
again (Endive checks `Thread.isInterrupted()` without clearing it), so the 
`WasmInterruptedException` came from the `dealloc` in `finally`, replacing the 
exception of the call, and the input buffer was never released. Any failure of 
that cleanup call replaced the original exception the same way.
   
   This change:
   
   - discards the instance when the call does not complete (any 
`RuntimeException` or `Error` from `alloc`, the memory write, the function, the 
read or `dealloc`): no further call is made into the failed instance, the 
exception of the failure is rethrown unchanged, and the next call creates a new 
instance from the already parsed module (under the same lock; about 0.1-0.2 ms 
for the 2.2 MB test module);
   - releases the error buffer with its size without the error bit, before 
throwing the error message as before;
   - keeps the instance when the function returns normally, including with the 
error bit set (the guest finished and its state is consistent).
   
   The rebuild is automatic, not behind an option: after a trap the instance is 
left in whatever state the guest had reached (a trap aborts the execution and 
nothing is rolled back, a half-updated allocator included; from the guest's own 
point of view that state is undefined), and the host cannot know what the guest 
allocated before failing, so there is no safe way to keep using it. The 
alternative is a `resetOnFailure`-style option (default `true`), for a guest 
that wants to keep state across a trap; I left it out because keeping a trapped 
instance is exactly the leak, but it is easy to add if reviewers prefer. 
Visible effect: state that a module keeps in its memory or globals between 
messages is reset after a failed call. That is documented in the component and 
language pages (new "Errors and module state" section, catalog copies 
regenerated) and in a 3-line upgrade-guide note `camel-wasm - a failed call 
discards the Wasm instance`.
   
   A small TLA+ model of `WasmFunction.run()` (exchanges sharing one instance, 
the guest allocator in two non-atomic steps, the shared operand stack, an abort 
while the guest runs, a sticky interrupt flag) shows the alternatives: with the 
lock and no abort it holds (no cross-talk, no double allocation, every block 
released when all exchanges are done, termination); an abort with a sticky flag 
leaks the input block; even with the flag cleared, the block the guest 
allocated before the abort leaks; discarding the aborted instance without 
calling into it holds, sticky flag or not (38,248 states). The lock itself is 
correct: no concurrency change here.
   
   Tests: new `WasmFunctionTest`. The guest is written in WAT in the test and 
compiled when the test runs with `run.endive:wabt` (new test-scoped dependency, 
same `${endive-version}` and project as the runtime, Apache-2.0), so no new 
binary is checked in. It has one page of memory and an arena allocator that 
gets its memory back only when every allocation was released, so a leak shows 
up as a later call that no longer fits; its `dealloc` traps on a size with the 
error bit set. The waits use Awaitility (no sleeps). Without the main-code 
change (two runs):
   ```
   WasmFunctionTest.testTrapDoesNotLeakGuestMemory:121 » Trap Trapped on 
unreachable instruction
   WasmFunctionTest.testErrorIsDeallocatedWithItsSize:130 Expecting actual 
throwable to be exactly an instance of:
     java.lang.RuntimeException
   but was:
     run.endive.runtime.TrapException: Trapped on unreachable instruction
   WasmFunctionTest.testInterruptDoesNotLeakGuestMemory:161 » Trap Trapped on 
unreachable instruction
   ```
   With the change the camel-wasm tests pass (9). No other module depends on 
camel-wasm (only the BOM, catalog and coverage aggregators).
   
   Not changed: the instance is still one per producer/expression, under one 
lock (a pool, a memory cap and a timeout are separate improvements; a timeout 
needs this change first). A `dealloc` that fails after the function returned an 
error is thrown instead of the error message, as before.
   
   # Target
   
   - [x] I checked that the commit is targeting the correct branch (Camel 4 
uses the `main` branch)
   
   # Tracking
   - [x] If this is a large change, bug fix, or code improvement, I checked 
there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for 
the change (usually before you start working on it).
   
   # Apache Camel coding standards and style
   
   - [x] I checked that each commit in the pull request has a meaningful 
subject line and body.
   - [ ] I have run `mvn clean install -DskipTests` locally from root folder 
and I have committed all auto-generated changes.
     (I built and tested the affected module with `-am`, including the 
formatter and import-sort plugins, and copied the changed docs to the catalog. 
I did not run the full root build.)
   
   # AI-assisted contributions
   
   - [x] If this PR includes AI-generated code, commits have proper 
co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR 
description identifies the AI tool used.
     This PR was prepared with Claude Code (Claude Opus 5.5). The commit 
carries a `Co-Authored-By` trailer.
   
   _Claude Code on behalf of allthingssecurity_
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to