Skip to content

Polyglot engine: script parameters leak across invocations, and timeout cancellation is cooperative with guest memory limits disabled #6759

Description

@ruispereira

Summary

Two resource/isolation gaps in the polyglot (JS) query engine, both admin-gated: script parameters leak across invocations because the shared context's bindings are never cleared, and a timed-out script is only cancelled cooperatively (with guest memory limits disabled entirely), so a non-interruptible call can wedge the shared engine.

Findings (all HEAD af4f67fd2)

1. Parameters/globals leak between commandsengine/src/main/java/com/arcadedb/query/polyglot/PolyglotQueryEngine.java:139-145:

synchronized (polyglotEngine) {
  if (parameters != null && !parameters.isEmpty()) {
    for (final Map.Entry<String, Object> entry : parameters.entrySet())
      polyglotEngine.setAttribute(entry.getKey(), entry.getValue());   // persistent binding, never removed
  }
  final Value result = polyglotEngine.eval(query);

PolyglotQueryEngine does not override QueryEngine.isReusable() (defaults true, QueryEngine.java:152) and LocalDatabase.getQueryEngine caches reusable engines (LocalDatabase.java:2003-2015), so one GraalPolyglotEngine.context — and its bindings — is shared by every caller on that database.
Repro: database.command("js", "return x;", Map.of("x", 5)) then database.command("js", "return x;", Map.of()) → the second returns 5 instead of a ReferenceError. Values (and any globals a script assigns) are visible to subsequent commands, including from a different caller.
Fix: remove the parameter members after each eval (e.g. bindings.removeMember(key) in a finally), or evaluate each command in a fresh child context — scoped so that registerFunctions' declared functions still survive, as intended.

2. Timeout cancellation is cooperative and there is no memory limitPolyglotQueryEngine.java:247-264 (executeUserCodefuture.cancel(true)), pool at :98-105 (8 threads, queue 10000, CallerRunsPolicy), per-command synchronized (polyglotEngine) at :139; limits at GraalPolyglotEngine.java:84-93:

// DISABLED LIMIT BECAUSE THE CONTEXT IS INVOKED MULTIPLE TIMES
//final ResourceLimits limits = ResourceLimits.newBuilder().statementLimit(10000, null).build();

Pure guest CPU loops are interrupted at GraalVM JS safepoints (covered by PolyglotQueryTest.timeout()), but a task blocked in a non-interruptible host call is not — and it holds the single polyglotEngine monitor that every command on the database serializes on. Eight such tasks fill the pool; CallerRunsPolicy then runs new work on the caller (HTTP worker) thread, which also blocks on the monitor. Separately, with ResourceLimits disabled, the CPU timeout does not bound memory: new Array(1e9) can OOM the JVM before any safepoint fires.
Fix: use GraalVM's supported cancellation (Context.interrupt(Duration) / context.close(true)) and/or per-execution ResourceLimits, and isolate contexts per execution so a wedged one can be discarded.

Scope / confidence

High on #1 (code read at HEAD; the reuse chain is explicit). Medium on #2's wedge (the interrupt path works for guest loops; the wedge needs a non-interruptible host call) — the disabled memory limit is deliberate and documented, noted because it means the timeout does not bound allocation. Both are gated behind UPDATE_SECURITY (DBA-level), so exposure is admin-only; filed as correctness/robustness, not a privilege issue.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions