eye-gu commented on code in PR #7582:
URL: https://github.com/apache/shenyu/pull/7582#discussion_r4272997308
##########
shenyu-plugin/shenyu-plugin-wasm-api/src/main/java/org/apache/shenyu/plugin/wasm/api/loader/WasmLoader.java:
##########
@@ -282,17 +374,66 @@ private Optional<ExportFunction> findWasmExtern(final
String funcName) {
return Optional.of(instance.export(funcName));
}
}
- LOG.debug("WASM export function '{}' not found in {}", funcName,
wasmName);
return Optional.empty();
}
-
+
+ private long[] executeOnPool(final String funcName, final long[] args) {
+ final Instance lease = acquire();
+ try {
+ return executeBound(lease, lease.export(funcName), args);
+ } finally {
+ release(lease);
+ }
+ }
+
+ // binds the thread to the instance so guest host callbacks resolve
getMemory() to it;
+ // pops on exit (nested calls each push their own entry)
+ private long[] executeBound(final Instance instance, final ExportFunction
export, final long[] args) {
+ final ArrayDeque<Instance> stack = bindings.get();
+ stack.push(instance);
+ try {
+ return export.apply(args);
+ } finally {
+ stack.pop();
Review Comment:
executeBound pops the binding in a finally on every call, so the ThreadLocal
never retains an Instance reference afterwards — what's left is one empty
ArrayDeque per thread. Calling bindings.remove() per pop would trade that
bounded ~100-byte retention for remove/re-create churn on the hot path, to
guard against a hypothetical future bug. Keeping as is.
##########
shenyu-plugin/shenyu-plugin-wasm-api/src/main/java/org/apache/shenyu/plugin/wasm/api/loader/WasmLoader.java:
##########
@@ -282,17 +374,66 @@ private Optional<ExportFunction> findWasmExtern(final
String funcName) {
return Optional.of(instance.export(funcName));
}
}
- LOG.debug("WASM export function '{}' not found in {}", funcName,
wasmName);
return Optional.empty();
}
-
+
+ private long[] executeOnPool(final String funcName, final long[] args) {
+ final Instance lease = acquire();
+ try {
+ return executeBound(lease, lease.export(funcName), args);
+ } finally {
+ release(lease);
+ }
+ }
+
+ // binds the thread to the instance so guest host callbacks resolve
getMemory() to it;
+ // pops on exit (nested calls each push their own entry)
+ private long[] executeBound(final Instance instance, final ExportFunction
export, final long[] args) {
+ final ArrayDeque<Instance> stack = bindings.get();
+ stack.push(instance);
+ try {
+ return export.apply(args);
+ } finally {
+ stack.pop();
+ }
+ }
+
+ private Instance acquire() {
+ final ArrayDeque<Instance> active = bindings.get();
+ if (active.size() >= poolSize) {
+ // nesting deeper than the pool: this thread already leases every
instance,
+ // so waiting could never succeed
+ throw new IllegalStateException(
+ "Nested export calls exceed the WASM instance pool size
for " + wasmName);
+ }
+ try {
+ return freeInstances.take();
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new IllegalStateException(
+ "Interrupted while waiting for a pooled WASM instance of "
+ wasmName, e);
+ }
+ }
Review Comment:
Unbounded take() is the intended backpressure of a fixed-size pool (same as
a JDBC connection pool): callers wait until an instance frees up, and release()
always runs in a finally, so the queue can't stay empty with no holders. A
timeout would just convert load spikes into 500s. Same-thread nesting is
already covered by the explicit guard in acquire().
--
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]
