SecureJavascriptConfigurator maxMemoryUsed does not work (accurately)

[ikaakkola]

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.

[jbarrez]

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.

[ikaakkola]

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.

[jbarrez]

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?

[ikaakkola]

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:

• keep track of allocation rate and provide a configurable limit to this

  • by keeping track of when observeInstructionCount was last called, one could use getThreadAllocatedBytes to calculate a "bytes allocated per second" value, which would be somewhat meaningful even when GC is not executed
  • trying to find suitable limits for this value would be rather difficult
  • probably would need to keep some kind of a rolling average for any limit enforcing to make any sense

• keep some sort of "Heap memory pressure" value available at all times and only enforce the "max memory used" value when X percent (configurable) of Heap memory has been used (average) for the last Y seconds (configurable)

  • so if 95% of heap has been in use for the last 5 seconds, the getThreadAllocatedBytes for the thread executing a script would be more like to be a sensible value ,as it would be expected for the JVM to attempt to perform GC as much as it can when Heap is running low
  • probably still not reliable, but possibly "good enough"
  • tricky to implement correctly

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..