davsclaus commented on code in PR #27593:
URL: https://github.com/apache/camel/pull/27593#discussion_r4227330867
##########
components/camel-wasm/src/main/docs/wasm-component.adoc:
##########
@@ -152,6 +152,18 @@ pub extern fn process(ptr: u32, len: u32) -> u64 {
}
----
+==== Errors and module state
+
+The module is instantiated once per endpoint (or per `wasm` expression) and
that instance handles every message, one at
Review Comment:
Small precision point: the instance is per producer rather than per
endpoint. Each `to("wasm:...")` step (and each producer a `ProducerTemplate`
creates) builds its own `WasmFunction` in `WasmProducer.doStart()`, so the same
endpoint used in two routes gets two instances that don't share state. Maybe:
"The module is instantiated once per producer (each `to` of a `wasm` endpoint
in a route has its own instance) and that instance handles every message, one
at a time, ...". On the language page, "once per `wasm` expression" is enough.
Please mirror it in the catalog copies.
##########
components/camel-wasm/src/main/java/org/apache/camel/wasm/WasmFunction.java:
##########
@@ -16,88 +16,119 @@
*/
package org.apache.camel.wasm;
+import java.nio.charset.StandardCharsets;
import java.util.Objects;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import run.endive.runtime.ExportFunction;
import run.endive.runtime.Instance;
import run.endive.wasm.WasmModule;
public class WasmFunction implements AutoCloseable {
+ private static final Logger LOG =
LoggerFactory.getLogger(WasmFunction.class);
+
private final Lock lock;
private final WasmModule module;
private final String functionName;
- private final Instance instance;
- private final ExportFunction function;
- private final ExportFunction alloc;
- private final ExportFunction dealloc;
+ // guarded by lock; null after a failed call until the next call creates a
new instance
+ private Instance instance;
+ private ExportFunction function;
+ private ExportFunction alloc;
+ private ExportFunction dealloc;
public WasmFunction(WasmModule module, String functionName) {
this.lock = new ReentrantLock();
this.module = Objects.requireNonNull(module);
this.functionName = Objects.requireNonNull(functionName);
- this.instance = Instance.builder(this.module).build();
- this.function = this.instance.export(this.functionName);
- this.alloc = this.instance.export(Wasm.FN_ALLOC);
- this.dealloc = this.instance.export(Wasm.FN_DEALLOC);
+ createInstance();
Review Comment:
Optional: these fields are no longer `final`, and the eager
`createInstance()` here runs outside the lock. Strictly by the Java memory
model, a first `run()` on another thread is then not guaranteed to see these
writes, which the `final` fields used to guarantee. In practice route startup
provides the happens-before, and the `instance == null` re-check covers a null
read, so this is only for robustness. Taking the lock here is enough:
```suggestion
lock.lock();
try {
createInstance();
} finally {
lock.unlock();
}
```
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc:
##########
@@ -2864,6 +2864,12 @@ Routes that deliberately read a `Camel`-prefixed header
produced by the Tika par
themselves after the `tika:parse` step, for example with a `setHeader` reading
the corresponding
non-prefixed metadata name.
+=== camel-wasm - a failed call discards the Wasm instance
Review Comment:
Heads-up: #27592 adds another camel-wasm entry (`=== camel-wasm - the wasm
predicate`) after the camel-xpath section. The two PRs merge without textual
conflicts, but the notes would end up separated by camel-xpath. Whichever
merges second, please put them next to each other, or under one `===
camel-wasm` heading.
--
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]
