-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
SecureJavascriptConfigurator maxMemoryUsed does not work (accurately) #3830
Comments
Would it be possible the share that example script? Reading the javadoc, I was under the impression that getThreadAllocatedBytes() would return the amount of allocated memory for that thread, taking into account any changes (and thus not simply counting up). However, from your observations it seems like that's not the case. |
ThreadMXBean does provide the currently allocated memory of the thread correctly yes. But, because the jvm uses GC to clean up allocated objects, the “allocated” value is inflated because it contains objects that would be freed by GC. If there is enough free heap available for the jvm, it will not run GC at all (or not aggressively, also depending on the chosen GC algorithm and the configuration for it). Thus, the value reported by threadmxbean is technically correct but does not reflect reality, a script can create 100s of megabytes of objects that it no longer references, but before garbage collection is ran, those show up as allocated memory. I will try to build a small sample testcase to show this behavior. |
Thanks for the explanation, that clarifies it. And besides manually allowing to trigger GC (which is not a guarantee) I indeed don't see an immediate solution to this, as it would require to "look if memory is deemed free-able" or to release memory, which is impossible on the JDK platform. No need for an example, it's clear, unless you see a potential solution to this? |
Yes, manually triggering GC is not a fix (and as you said, nothing guarantees it will happen immediately, or at all). I have no immediate solution to this either, some ideas, but neither of these is really reliable / do what the current implementation claims to do:
Flowable is not the only Rhino sandbox using the exact same logic to limit memory for a script, but none of them seem to actually take GC into account at all.. |
Describe the bug
When running Script tasks and enabling maxMemoryUsed the underlying Javascript engine (Rhino) will periodically call observeInstructionCount in SecureScriptContextFactory which then uses (JVM specific) ThreadMXBean to calculate the currently allocated bytes for the thread executing the script.
This all is technically correct, but does not take garbage collection into account in any way, leading to the reported 'allocated bytes' value to be (sometimes significantly) higher than retained bytes for the script after a garbage collection is performed. As long as the JVM has enough heap space available, it might choose to not perform garbage collection at all.
If System.gc() is forced on every _ observeInstructionCount_ call, a test script (busy loop that just allocates some variables inside the loop) runs for minutes before reaching the memory limit, while without forcing a gc the script terminates within seconds (with a memory limit of 128MB).
Expected behavior
The script is terminated only when it uses more memory than the configured limit, taking garbage collection into account.
I believe this is not possible to implement. If nothing else, the documentation for this feature should clearly state that it is not used memory but more close to allocated memory that is limited.
Additional context
Flowable 7.0.1, with Spring Boot
Also worthy of note, the current implementation will report -1 when used with Java 21 Virtual threads ; ThreadMXBean.getThreadAllocatedBytes(threadId) does not support virtual threads.
The text was updated successfully, but these errors were encountered: