vyommani commented on PR #1267: URL: https://github.com/apache/ranger/pull/1267#issuecomment-5905240758
here is my suggestion how to address this issue The finally around evaluation is the right place to release resources, but ScriptEngine.close() does not free the Graal objects this path allocates. GraalScriptEngineCreator calls GraalJSScriptEngine.create(null, sharedBuilder). A null engine makes Graal allocate a new org.graalvm.polyglot.Engine on every call, and create() also stores that engine on the shared Context.Builder. RangerRequestScriptEvaluator then uses createBindings() and eval(script, bindings). Those bindings own a separate polyglot Context. On js-scriptengine 25.1.3, GraalJSScriptEngine.close() closes only the engine's default ScriptContext. It does not close the bindings context that eval used, and it does not close the private engine. Each evaluation still leaks both, and close() builds a second context on the shared builder just to close the unused default one. Graal's model is one shared Engine, a new Context per evaluation, and close that context: • Keep one Engine in GraalScriptEngineCreator and pass it to GraalJSScriptEngine.create(sharedEngine, builder). Build a new Context.Builder per engine from the existing HostAccess. The builder is mutated by option(...) and engine(...), so it cannot be shared across request threads. The HostAccess and the Engine can. • Close the bindings after each evaluation, including when evaluation throws. GraalJSBindings is AutoCloseable and its close() closes the context eval used. RangerRequestScriptEvaluator owns those bindings, and both resource expressions and script conditions create a new evaluator per use, so the close belongs there. • Leave the shared Engine open. Do not call getPolyglotEngine().close() for every ScriptEngine: an engine from ScriptEngineManager shares one factory engine, and closing it would break later evaluations. The Nashorn path can stay per call. It is not AutoCloseable and is not safe to share across threads. The current tests mock closeScriptEngine and a plain ScriptEngine, so they stay green when the real context and engine remain open. Please add a test with a real GraalJSScriptEngine from this creator that checks the bindings context is closed, the shared engine still evaluates a second expression, and the same holds when evaluation throws. -- 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]
