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

Hard crash if out of memory #84

Closed
erik-kallen opened this issue Nov 17, 2018 · 23 comments
Closed

Hard crash if out of memory #84

erik-kallen opened this issue Nov 17, 2018 · 23 comments
Assignees

Comments

@erik-kallen
Copy link

First, thank you for this project. We use it extensively to do server-side rendering of our React application and it works really well.

One issue we have encountered (which I don't if it is possible to fix) is that when we had a memory leak, we would eventually get a hard process crash with this call stack:

Application: w3wp.exe
Framework Version: v4.0.30319
Description: The process was terminated due to an unhandled exception.
Exception Info: System.AccessViolationException
   at <Module>.std._Func_class<void>.()()(std._Func_class<void>*)
   at Microsoft.ClearScript.V8.NativeCallbackImpl.Invoke()
   at Microsoft.ClearScript.Util.MiscHelpers.Try(System.Action)
   at Microsoft.ClearScript.Util.MiscHelpers+<>c__DisplayClass23_0.<QueueNativeCallback>b__0(System.Object)
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem()
   at System.Threading.ThreadPoolWorkQueue.Dispatch()

The error can be triggered by this code:

var runtime = new V8Runtime();
var engine = runtime.CreateScriptEngine(V8ScriptEngineFlags.DisableGlobalMembers);
engine.AllowReflection = false;
engine.EnableAutoHostVariables = false;
engine.UseReflectionBindFallback = false;
engine.DefaultAccess = ScriptAccess.None;
engine.Execute("var leak = [];");
for (;;)
{
    engine.Execute("(function() {const x = []; for (let i = 0; i < 10000; i++) x[i] = Math.random(); leak.push(x);})();");
}

I am aware that my code needs to be fixed and that a crash is inevitable, but I would prefer if the crash happened in some other way than an access violation (which I guess is due to an allocation return value not being checked somewhere) that tears down my entire process without the possibility of preventing it.

Preferrably I would eventually get an exception in the Execute() call (which could or could not require that the engine be disposed of afterwards).

@ClearScriptLib
Copy link
Collaborator

ClearScriptLib commented Nov 18, 2018

Hi @erik-kallen,

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.

The bottom line is that unless you're using V8 to run trusted, well-tested script code, you're advised to use a dedicated process whose sudden termination your application can withstand. In a critical process, running untrusted script code is just as risky as calling into an unknown shared library. Some script engines mitigate this risk by running script code in a sandbox or jail. V8 does not.

That said, ClearScript allows you to set a "soft limit" on V8 memory consumption. See MaxHeapSize and MaxRuntimeHeapSize.

V8's default limit appears to be about 1.5 GB in a 64-bit process (and much lower in 32-bit). Here's your sample with modifications that demonstrate OOM recovery via memory consumption monitoring:

var runtime = new V8Runtime(new V8RuntimeConstraints { MaxOldSpaceSize = 3072 });
runtime.MaxHeapSize = (UIntPtr)(2048UL * 1024 * 1024);
var engine = runtime.CreateScriptEngine();
try {
    engine.Execute("var leak = [];");
    for (;;) {
        engine.Execute("(function() {const x = []; for (let i = 0; i < 10000; i++) x[i] = Math.random(); leak.push(x);})();");
    }
}
catch (ScriptEngineException exception) {
    Console.WriteLine(exception.GetBaseException().Message);
}

This example terminates script execution at approximately 2GB, exceeding V8's default.

Note that memory consumption monitoring is inexact, and it slows down script execution. You can tune its aggressiveness via HeapSizeSampleInterval or RuntimeHeapSizeSampleInterval. And since it operates periodically, it may fail if memory consumption spikes suddenly between samples.

Good luck!

@erik-kallen
Copy link
Author

Thank you for the explanation!

Would this be the same as me checking the result of GetHeapInfo().UsedHeapSize periodically?

How does it play with garbage collection? I guess that the UsedHeapSize (which I guess is what the MaxHeapSize check uses) includes objects that are not yet garbage collected? I have noticed that even if I do CollectGarbage(false) right before I call GetHeapInfo(), the result differ by a factor of 2 or so between calls even if I have no leak.

@ClearScriptLib
Copy link
Collaborator

ClearScriptLib commented Nov 19, 2018

Would this be the same as me checking the result of GetHeapInfo().UsedHeapSize periodically?

ClearScript uses TotalHeapSize, but yes, that's basically how it works.

As long as the engine is busy, ClearScript periodically schedules a callback on the script execution thread. The callback checks V8's heap size, collects garbage aggressively if memory consumption exceeds the limit, and terminates script execution if garbage collection wasn't sufficiently effective.

@erik-kallen
Copy link
Author

If I read the docs (v8 + clearscript) correctly, it seems that TotalHeapSize will never be decreased, is that correct?

I noticed that in your example. you used MaxOldSpaceSize in the V8Constraints to set the limit to a value higher than the MaxHeapSize, do you have any recommendations for MaxNewHeapSize?

@erik-kallen
Copy link
Author

BTW, do we get a notification somehow so it would be possible to add something to the event log in the case of OOM? I have noticed that I sometimes get information there, and sometimes not.

@ClearScriptLib
Copy link
Collaborator

Hi @erik-kallen,

If I read the docs (v8 + clearscript) correctly, it seems that TotalHeapSize will never be decreased, is that correct?

That's not correct. V8 releases unused committed memory during exhaustive garbage collection. Here's how you can verify that:

Console.WriteLine(engine.GetRuntimeHeapInfo().TotalHeapSize);
engine.Execute("var x = []; for (var i = 0; i < 50000000; ++i) x.push(i);");
Console.WriteLine(engine.GetRuntimeHeapInfo().TotalHeapSize);
engine.Execute("x = undefined;");
engine.CollectGarbage(true);
Console.WriteLine(engine.GetRuntimeHeapInfo().TotalHeapSize);

I noticed that in your example. you used MaxOldSpaceSize in the V8Constraints to set the limit to a value higher than the MaxHeapSize

Yes. Setting MaxOldSpaceSize significantly higher than MaxHeapSize is critical, as it makes monitoring much more likely to terminate script execution before V8 kills the process.

do you have any recommendations for MaxNewHeapSize [sic]?

No. That doesn't seem to have much effect in recent V8 versions, whereas careful setting of MaxOldSpaceSize and Max[Runtime]HeapSize can prevent most hard crashes.

BTW, do we get a notification somehow so it would be possible to add something to the event log in the case of OOM?

V8 does allow the host to set up an OOM callback (for logging purposes only; it'll kill the process as soon as the callback returns), but ClearScript doesn't expose this. Of course, if you use the techniques discussed here, you can catch OOM before it kills the process and log anything you wish.

Cheers!

@erik-kallen
Copy link
Author

Thank you for your help. You can close this issue, if you want, or leave it open for future readers

@ClearScriptLib
Copy link
Collaborator

ClearScript's heap and stack monitoring features were developed specifically to help avoid V8 OOM crashes. Please reopen this issue if you have suggestions for improving them. Thanks!

@erik-kallen
Copy link
Author

The only suggestion I have is to add something to the eventlog or similar in the notification from V8. Most crashes due to this error seem to not leave any trace

@ClearScriptLib
Copy link
Collaborator

Hmm. Windows should generate an event for all process crashes. We'll take a look.

@erik-kallen
Copy link
Author

It does, but it is not always that it includes a stacktrace that includes ClearScript or v8

@ClearScriptLib
Copy link
Collaborator

The problem is very likely to be that V8's JIT-compiled code produces non-standard stack frames that confuse the system crash handler. And since V8 APIs for walking the script stack won't work post-OOM, a custom crash handler would be powerless to produce better results.

@erik-kallen
Copy link
Author

I am not especially interested in the v8 stack frame, we just had to spend a lot of time guessing in the dark because we usually didn't get either the words "ClearScript" or "v8" in the error event. Either of those words would have pointed us in the right direction

@ClearScriptLib
Copy link
Collaborator

A custom V8 crash handler couldn't prevent the system from generating such an event. The best it could do is generate another event that you could recognize. Would that be helpful?

@erik-kallen
Copy link
Author

I think that would be helpful, yes. But don't do it for me, my problems are already solved :)

@antibarbie
Copy link

Hello, Does that mean that such a code be sufficient to trigger the detection before the OOM handler ? (Provided there are no spikes in memory usage)

m_engine = new V8ScriptEngine(instanceName);

// ... code omitted ...

// Trigger a custom memory detection before V8 out-of-memory handler
var rtinfo = m_engine.GetRuntimeHeapInfo();
m_engine.MaxRuntimeHeapSize = new UIntPtr((uint)(rtinfo.HeapSizeLimit * 0.95f));

@ClearScriptLib
Copy link
Collaborator

ClearScriptLib commented Apr 10, 2019

Hi @antibarbie,

That might work, although the default seems rather arbitrary, especially in 64-bit environments, where you should be able to use a significantly larger limit.

Unfortunately, heap size monitoring cannot be made bulletproof. If the script code spikes memory usage suddenly, heap size monitoring may not be able to catch it. You can adjust its aggressiveness, but it's currently capped at 4 samples per second.

Good luck!

@anyway2019
Copy link

anyway2019 commented Feb 24, 2021

i came into same issue,it seems a little different, i set the MaxHeapSize as above but the problem still come out at times.
the stacktrace :

 System.AccessViolationException
   在 Microsoft.ClearScript.V8.SplitProxy.V8SplitProxyNative+Impl_Windows_X86.NativeCallback_Invoke(Handle)
   在 Microsoft.ClearScript.V8.SplitProxy.V8SplitProxyNative+Impl_Windows_X86.Microsoft.ClearScript.V8.SplitProxy.IV8SplitProxyNative.NativeCallback_Invoke(Handle)
   在 Microsoft.ClearScript.V8.SplitProxy.NativeCallbackImpl.<Invoke>b__4_0(Microsoft.ClearScript.V8.SplitProxy.IV8SplitProxyNative)
   在 Microsoft.ClearScript.V8.SplitProxy.V8SplitProxyNative.InvokeNoThrow(System.Action`1<Microsoft.ClearScript.V8.SplitProxy.IV8SplitProxyNative>)
   在 Microsoft.ClearScript.V8.SplitProxy.NativeCallbackImpl.Invoke()
   在 Microsoft.ClearScript.Util.MiscHelpers.Try(System.Action)
   在 Microsoft.ClearScript.Util.MiscHelpers+<>c__DisplayClass33_0.<QueueNativeCallback>b__0(System.Object)
   在 System.Threading.QueueUserWorkItemCallback.WaitCallback_Context(System.Object)
   在 System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   在 System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   在 System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem()
   在 System.Threading.ThreadPoolWorkQueue.Dispatch()
   在 System.Threading._ThreadPoolWaitCallback.PerformWaitCallback()

@ClearScriptLib
Copy link
Collaborator

Hi @lishuo0710,

This looks like the same issue. The stack trace is different due to the recent elimination of managed C++ code.

We refer you to this comment from above:

Unfortunately, heap size monitoring cannot be made bulletproof. If the script code spikes memory usage suddenly, heap size monitoring may not be able to catch it. You can adjust its aggressiveness, but it's currently capped at 4 samples per second.

Note that newer ClearScript versions can monitor heap size at up to 8 samples per second. See V8ScriptEngine.RuntimeHeapSizeSampleInterval.

Thanks!

@anyway2019
Copy link

so the crash is due to the js script?
i use turf.js from local js file and for high perfomance i put V8ScriptEngine in global variable and compile the js file once,so what i can do is change the way to use v8?

@ClearScriptLib
Copy link
Collaborator

Hi @lishuo0710,

so the crash is due to the js script?

It's difficult to tell from a managed-only call stack. During script execution, V8 often instructs the host to invoke a callback on a background worker thread. All we can tell from your stack trace is that one of these callbacks crashed. A full trace with native call frames might be more revealing.

In any case, the purpose of these callbacks isn't well documented, but they appear to be mostly related to heap management – background garbage collection and such. Crashes within them are usually indicative of heap exhaustion.

Do you have a minimal reliable repro case for this issue? If not, try some or all of the following:

  1. Use V8RuntimeConstraints to expand your V8 heap limits.
  2. Stop memory leaks by periodically starting over with a fresh V8ScriptEngine instance. Don't forget to dispose the old one.
  3. Host V8 in a 64-bit process.

Good luck!

@anyway2019
Copy link

Hi @lishuo0710,

so the crash is due to the js script?

It's difficult to tell from a managed-only call stack. During script execution, V8 often instructs the host to invoke a callback on a background worker thread. All we can tell from your stack trace is that one of these callbacks crashed. A full trace with native call frames might be more revealing.

In any case, the purpose of these callbacks isn't well documented, but they appear to be mostly related to heap management – background garbage collection and such. Crashes within them are usually indicative of heap exhaustion.

Do you have a minimal reliable repro case for this issue? If not, try some or all of the following:

  1. Use V8RuntimeConstraints to expand your V8 heap limits.
  2. Stop memory leaks by periodically starting over with a fresh V8ScriptEngine instance. Don't forget to dispose the old one.
  3. Host V8 in a 64-bit process.

Good luck!

Thanks! i will try it.

@ClearScriptLib
Copy link
Collaborator

Hi @lishuo0710,

Please consider giving Version 7.1.1 a try. It includes a potential fix for the crash you're seeing as well as a new feature that helps avoid OOM issues.

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants