Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ScriptProfile: Recover from closed context for JS Scripting #4437

Conversation

florian-h05
Copy link
Contributor

@florian-h05 florian-h05 commented Nov 2, 2024

From time to time, I can see the following exception in my logs:

... AbstractInvocationHandler] - An exception occurred while calling method 'Profile.onStateUpdateFromItem()' on '...ScriptProfile@...': The Context is already closed.
java.lang.IllegalStateException: The Context is already closed.
        at com.oracle.truffle.polyglot.PolyglotEngineException.illegalState(PolyglotEngineException.java:129) ~[?:?]
        at com.oracle.truffle.polyglot.PolyglotContextImpl.checkClosed(PolyglotContextImpl.java:1103) ~[?:?]
        at com.oracle.truffle.polyglot.PolyglotContextImpl.enterThreadChanged(PolyglotContextImpl.java:702) ~[?:?]
        at com.oracle.truffle.polyglot.PolyglotEngineImpl.enterCached(PolyglotEngineImpl.java:1991) ~[?:?]
        at com.oracle.truffle.polyglot.HostToGuestRootNode.execute(HostToGuestRootNode.java:110) ~[?:?]
        at com.oracle.truffle.api.impl.DefaultCallTarget.callDirectOrIndirect(DefaultCallTarget.java:85) ~[?:?]
        at com.oracle.truffle.api.impl.DefaultCallTarget.call(DefaultCallTarget.java:102) ~[?:?]
        at com.oracle.truffle.polyglot.PolyglotMap.get(PolyglotMap.java:127) ~[?:?]
        at com.oracle.truffle.polyglot.PolyglotMap.put(PolyglotMap.java:133) ~[?:?]
        at com.oracle.truffle.js.scriptengine.GraalJSBindings.put(GraalJSBindings.java:130) ~[?:?]
        at javax.script.SimpleScriptContext.setAttribute(SimpleScriptContext.java:246) ~[java.scripting:?]
        at org.openhab.core.automation.module.script.ScriptTransformationService.transform(ScriptTransformationService.java:187) ~[?:?]
        at org.openhab.core.automation.module.script.profile.ScriptProfile.executeScript(ScriptProfile.java:185) ~[?:?]
        at org.openhab.core.automation.module.script.profile.ScriptProfile.transformState(ScriptProfile.java:173) ~[?:?]
        at org.openhab.core.automation.module.script.profile.ScriptProfile.onStateUpdateFromHandler(ScriptProfile.java:153) ~[?:?]
        at org.openhab.core.thing.internal.CommunicationManager.lambda$14(CommunicationManager.java:508) ~[?:?]
        at org.openhab.core.thing.internal.CommunicationManager.lambda$17(CommunicationManager.java:543) ~[?:?]
        at java.lang.Iterable.forEach(Iterable.java:75) ~[?:?]
        at org.openhab.core.thing.internal.CommunicationManager.handleCallFromHandler(CommunicationManager.java:539) ~[?:?]
        at org.openhab.core.thing.internal.CommunicationManager.stateUpdated(CommunicationManager.java:506) ~[?:?]
        at org.openhab.core.thing.internal.ThingHandlerCallbackImpl.stateUpdated(ThingHandlerCallbackImpl.java:66) ~[?:?]
        at org.openhab.core.thing.binding.BaseThingHandler.updateState(BaseThingHandler.java:270) ~[?:?]
        at org.openhab.binding.astro.internal.handler.AstroThingHandler.publishChannelIfLinked(AstroThingHandler.java:178) ~[?:?]
        at org.openhab.binding.astro.internal.handler.AstroThingHandler.publishPlanet(AstroThingHandler.java:160) ~[?:?]
        at org.openhab.binding.astro.internal.handler.SunHandler.publishPositionalInfo(SunHandler.java:66) ~[?:?]
        at org.openhab.binding.astro.internal.job.PositionalJob.run(PositionalJob.java:43) ~[?:?]
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572) ~[?:?]
        at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:358) ~[?:?]
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:305) ~[?:?]
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144) ~[?:?]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642) ~[?:?]
        at java.lang.Thread.run(Thread.java:1583) [?:?]
Caused by: com.oracle.truffle.api.TruffleStackTrace$LazyStackTrace

@florian-h05 florian-h05 requested a review from a team as a code owner November 2, 2024 12:29
@florian-h05 florian-h05 force-pushed the script-profile-context-already-closed branch from 4793ec9 to 466489d Compare November 3, 2024 00:01
Signed-off-by: Florian Hotze <[email protected]>
@florian-h05
Copy link
Contributor Author

@J-N-K The ScriptTransformationService should now recover from already closed context.

@florian-h05 florian-h05 changed the title ScriptProfile: Attempt to fix ISE due to Context is already closed ScriptProfile: Recover from closed context, fix ISE Nov 3, 2024
…a/org/openhab/core/automation/module/script/ScriptTransformationService.java

Co-authored-by: J-N-K <[email protected]>
Signed-off-by: Florian Hotze <[email protected]>
@florian-h05
Copy link
Contributor Author

@J-N-K I addressed your review through GitHub web, FYI DCO is now failing.

@holgerfriedrich
Copy link
Member

@florian-h05 Evaluating the exception text generated by an external library might fail in the future. Not too bad, but the ISE might be back. Worth to write a test which tries to provoke the ISE and checks the error message?
Up to you, I am also fine to merge it as it is.

@florian-h05
Copy link
Contributor Author

Worth to write a test which tries to provoke the ISE and checks the error message?

That would require to have the JS Scripting add-on available. I don‘t know how feasible that is, never have written an itest in core before.

In any case, I should add a comment to the code explaining the message.

…a/org/openhab/core/automation/module/script/ScriptTransformationService.java

Signed-off-by: Florian Hotze <[email protected]>
@florian-h05
Copy link
Contributor Author

I will add a comment to JS Scripting‘s POM as a reminder to check if this fix still works when upgrading GraalJS.

@florian-h05
Copy link
Contributor Author

@holgerfriedrich I would merge it as is as I think it would be a larger effort to write an itest including JS Scripting to provoke the exception.

@florian-h05 florian-h05 changed the title ScriptProfile: Recover from closed context, fix ISE ScriptProfile: Recover from closed context for JS Scripting Nov 4, 2024
@holgerfriedrich
Copy link
Member

LGTM.

@J-N-K do you want to merge (just asking because of the broken DCO)

@florian-h05
Copy link
Contributor Author

florian-h05 commented Nov 4, 2024

DCO is broken because GitHub web expects my old mail address ([email protected]) (which still works) instead of the new one ([email protected]).

florian-h05 added a commit to florian-h05/openhab-addons that referenced this pull request Nov 5, 2024
@holgerfriedrich holgerfriedrich merged commit 4eec4a3 into openhab:main Nov 6, 2024
5 checks passed
@holgerfriedrich holgerfriedrich added the bug An unexpected problem or unintended behavior of the Core label Nov 6, 2024
@holgerfriedrich holgerfriedrich added this to the 4.3 milestone Nov 6, 2024
@florian-h05 florian-h05 deleted the script-profile-context-already-closed branch November 6, 2024 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants