microsoft / ClearScript

A library for adding scripting to .NET applications. Supports V8 (Windows, Linux, macOS) and JScript/VBScript (Windows).
https://microsoft.github.io/ClearScript/
MIT License
1.74k stars 148 forks source link

Runtime exit gracefully when reached memory limit #499

Closed JangoBoogaloo closed 1 year ago

JangoBoogaloo commented 1 year ago

Hi I am testing executing Java script with insecure memory usage (huge allocation). I realized that I can set the runtime constraints. However when it reaches the limit, the runtime will abort with stack trace. Is there a way I can let the runtime exit gracefully without aborting my application and logging the Java script stacktrace?

Exit code is 133 (Warning: disabling flag --expose_wasm due to conflicting flags

<--- Last few GCs --->

[37623:0x7fec300b0000]      103 ms: Mark-Compact 21.8 (54.3) -> 21.8 (54.3) MB, 12.7 / 0.0 ms  (+ 0.2 ms in 2 steps since start of marking, biggest step 0.1 ms, walltime since start of marking 13 ms) (average mu = 0.345, current mu = 0.345) finalize incre[37623:0x7fec300b0000]      133 ms: Mark-Compact 30.1 (62.8) -> 30.1 (62.8) MB, 23.7 / 0.0 ms  (average mu = 0.257, current mu = 0.199) allocation failure; scavenge might not succeed

<--- JS stacktrace --->

#
# Fatal javascript OOM in Reached heap limit
#)

Last runner error: Process /usr/local/share/dotnet/dotnet:37623 exited with code '133': Warning: disabling flag --expose_wasm due to conflicting flags <--- Last few GCs ---> [37623:0x7fec300b0000] 103 ms: Mark-Compact 21.8 (54.3) -> 21.8 (54.3) MB, 12.7 / 0.0 ms (+ 0.2 ms in 2 steps since start of marking, biggest step 0.1 ms, walltime since start of marking 13 ms) (average mu = 0.345, current mu = 0.345) finalize incre[37623:0x7fec300b0000] 133 ms: Mark-Compact 30.1 (62.8) -> 30.1 (62.8) MB, 23.7 / 0.0 ms (average mu = 0.257, current mu = 0.199) allocation failure; scavenge might not succeed <--- JS stacktrace ---> # # Fatal javascript OOM in Reached heap limit #

My Code:

        try
        {
            var constraints = new V8RuntimeConstraints
            {
                MaxArrayBufferAllocation = 30,
                MaxNewSpaceSize = 30,
                MaxOldSpaceSize = 30,
            };
            using var engine = new V8ScriptEngine(constraints);
            engine.Execute("function allocateMemory() { array = []; for(let i = 0; i <= 999999; i++) { array.push((new Array(99999)).fill('aaaa')); } }");
            engine.Script.allocateMemory();
        }
        catch (ScriptEngineException ex)
        {
            await System.Console.Error.WriteLineAsync(ex.ErrorDetails);
        }
ClearScriptLib commented 1 year ago

Hi @JangoBoogaloo,

Unfortunately, V8 crashes the process if it fails to allocate memory and garbage collection doesn't help. It is the position of the V8 team that this behavior is appropriate given V8's goals. See here for some relevant discussion. Some quotes:

If GC fails three times in a row to allocate, the process dies.

The problem [...] is that one would still need to do some dynamic checks in the generated code to see if OOM happened or not. We don't want to do that for performance reasons, because all you can normally do is crash anyway.

I can understand that there are scenarios outside of Chrome where a slower execution handling OOM more gracefully would be preferable, but this is outside v8's scope.

That said, ClearScript allows you to set a "soft limit" on V8 memory consumption. See MaxHeapSize and MaxRuntimeHeapSize. This limit is enforced via periodic sampling, which can't always catch sudden spikes. However, in conjunction with runtime constraints and the recently added HeapExpansionMultiplier, it should provide something close to the desired behavior.

Here's your sample code with modifications that demonstrate the technique:

try {
    var constraints = new V8RuntimeConstraints {
        MaxArrayBufferAllocation = 30,
        MaxNewSpaceSize = 30,
        MaxOldSpaceSize = 30,
        HeapExpansionMultiplier = 1.1
    };
    using (var engine = new V8ScriptEngine(constraints)) {
        engine.MaxRuntimeHeapSize = (UIntPtr)((constraints.MaxNewSpaceSize + constraints.MaxOldSpaceSize) * 1024 * 1024);
        engine.Execute("function allocateMemory() { array = []; for(let i = 0; i <= 999999; i++) { array.push((new Array(99999)).fill('aaaa')); } }");
        engine.Script.allocateMemory();
    }
}
catch (ScriptEngineException ex) {
    await Console.Error.WriteLineAsync(ex.ErrorDetails);
}

Interestingly, the V8 team has apparently recognized the need for real V8 sandboxing and is working on it. Unfortunately, the V8 sandbox is currently experimental, and ClearScript does not yet support it.

Good luck!

TechPizzaDev commented 1 year ago

What about v8::Isolate::SetFatalErrorHandler and v8::Isolate::SetOOMErrorHandler? They seem to at least allow the host to try process the exception without aborting, even if it leaves the isolate unusable afterwards.

ClearScriptLib commented 1 year ago

They seem to at least allow the host to try process the exception without aborting

Sadly, no. Those callbacks only allow the host to perform custom logging or cleanup in preparation for process termination. If the handler returns, V8 kills the process anyway (example here).

TechPizzaDev commented 1 year ago

Ah that's funny. I am circumventing that FATAL termination by throwing an exception in the callback which gets propagated up to C#. I do have a problem where continuation timers try to tick and access a locked-out isolate.

And I tuned into this discussion since I'm working on a V8-only edition of ClearScript that will only have source-generated bindings and no reflection at runtime.

ClearScriptLib commented 1 year ago

I am circumventing that FATAL termination by throwing an exception in the callback which gets propagated up to C#.

We tried that a while back – exceptions, longjmp, etc. – and some attempts seemed to work, at least on the platforms supported at the time, but that approach seemed fraught with risk, as V8 is compiled without exception support, not to mention that its JIT frames aren't necessarily standard.

And I tuned into this discussion since I'm working on a V8-only edition of ClearScript that will only have source-generated bindings and no reflection at runtime.

That sounds like an interesting project! We're curious:

TechPizzaDev commented 1 year ago

While I don't have an answer to how stable throwing an exception is in the middle of execution, I can answer the two points.

How can you avoid reflection if you need to invoke a managed method by name?

I will be writing a source-generator that exposes managed classes in the form of V8 templates, using function pointers to leverage allocation-less interop from JS to C#. The C# to JS interop generator will generate fast POCO-to-V8Value conversions. This is all theoretical for now, and I will be writing some demos in the coming months.

We've never contributed, yet your repository attributes some commits to ClearScriptLib

I was trying to debug why V8Build.cmd exited early (turned out to be missing calls, I might PR that to ClearScript), and it seems that the git config that is used for applying patches leaked into my repository config.

ClearScriptLib commented 1 year ago

I will be writing a source-generator that exposes managed classes in the form of V8 templates, using function pointers to leverage allocation-less interop from JS to C#. The C# to JS interop generator will generate fast POCO-to-V8Value conversions.

That sounds fantastic!

I was trying to debug why V8Build.cmd exited early (turned out to be missing calls, I might PR that to ClearScript)

No need to PR it; we already have that change on deck for the next release. Apparently a DepotTools update changed the ninja executable to a batch script 🙄

the git config that is used for applying patches leaked into my repository config

Ah, that makes sense. Cheers!

ClearScriptLib commented 1 year ago

Please reopen this issue if you have additional thoughts or questions about this topic. Thank you!