Skip to content

Block finalizer solved? #18

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

Closed
comprosedev opened this issue May 23, 2018 · 8 comments
Closed

Block finalizer solved? #18

comprosedev opened this issue May 23, 2018 · 8 comments

Comments

@comprosedev
Copy link

comprosedev commented May 23, 2018

After doing an analysis of one of our Memory dumps, Microsoft stated this.

_However for your specific instance your finalizer thread is blocked by a ChakraEdgeJsRtJsEngine object. When looking at the amount of exceptions it seems as if there are thousands and thousands of exceptions all rooted to System.Net.Connection which are all stuck waiting to be finalized. They can't execute their finalize method because of the blocked finalizer thread below.

It's stuck on a Wait call which is part of the Dispose() method. I would recommend disposing ALL instances of the Chakra engine in your code as this will explicitly avoid the finalizer from being called. What the ChakraEngine looks like it's doing here is when this object is finalized it called a native method to reduce a ref counter to an object, this is either taking too long or never returns (I can't tell).

            Assuming you're using this library here (https://github.com/Taritsyn/MsieJavaScriptEngine), I would take a look at this code to ensure you know what it's doing when, as there are a lot of finalizers due to the PInvokes into the JavaScript engine. I think it's also pretty clear that this isn't a Microsoft library so I don't know how far Michael or I can help debug this._

Here is what is it waiting on.
[[HelperMethodFrame_1OBJ] (System.Threading.WaitHandle.WaitOneNative)] System.Threading.WaitHandle.WaitOneNative(System.Runtime.InteropServices.SafeHandle, UInt32, Boolean, Boolean)

mscorlib_ni!System.Threading.WaitHandle.InternalWaitOne(System.Runtime.InteropServices.SafeHandle, Int64, Boolean, Boolean)+1b
mscorlib_ni!System.Threading.WaitHandle.WaitOne(Int32, Boolean)+2e
MsieJavaScriptEngine.ScriptDispatcher.InnnerInvoke(System.Func`1)+80
MsieJavaScriptEngine.JsRt.Edge.ChakraEdgeJsRtJsEngine.Dispose(Boolean)+58
MsieJavaScriptEngine.JsRt.Edge.ChakraEdgeJsRtJsEngine.Finalize()+d
[[DebuggerU2MCatchHandlerFrame]]
[[ContextTransitionFrame]]
[[GCFrame]]
[[DebuggerU2MCatchHandlerFrame]]

Do you know if this is fixed in your latest release?

@Taritsyn
Copy link
Owner

Do you know if this is fixed in your latest release?

What version of the MSIE JavaScript Engine are you using? Do you use this library as part of some other product?

@comprosedev
Copy link
Author

We are currently using 2.2.7. I downloaded the code and made this change and it seems that the finalizer is no longer being blocked. Removed the invoke.

ChakraEdgeJsRtJsEngine.cs:
protected override void Dispose(bool disposing)
{
try
{
if (_disposedFlag.Set())
{
if (_dispatcher != null)
{
if (_jsContext.IsValid)
{
_jsContext.Release();
}

                    _jsRuntime.Dispose();
                    _dispatcher.Dispose();
                }
                base.Dispose(disposing);
            }
        }
        catch (Exception e)
        {
            base.Dispose(disposing);
        }
    }

@Taritsyn
Copy link
Owner

Why do you use the following construction?

try
{}
catch (Exception e)
{
	base.Dispose(disposing);
}

@comprosedev
Copy link
Author

I have always been in the habit of making sure that finalizer methods handle all exceptions.
I noticed that the Release method has:
EdgeJsErrorHelpers.ThrowIfError(EdgeNativeMethods.JsContextRelease(this, out count));

Perhaps this is extraneous though?

@Taritsyn
Copy link
Owner

Perhaps this is extraneous though?

Yes, it is extraneous!

At the weekend, I will try to assess all the risks, that may be caused by the removal of _dispatcher.Invoke.

@comprosedev
Copy link
Author

Much appreciated!

@Taritsyn
Copy link
Owner

Try to upgrade to version 2.2.8.

@comprosedev
Copy link
Author

Awesome, thanks for the quick fix!

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

No branches or pull requests

2 participants