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

AccessViolationException #1033

Closed
Havret opened this issue May 24, 2016 · 13 comments
Closed

AccessViolationException #1033

Havret opened this issue May 24, 2016 · 13 comments

Comments

@Havret
Copy link

Havret commented May 24, 2016

Hi!

When I try recreate Chakra Runtime, create Chakra Context and acquire Context Scope several times I got the following exception when trying to call a function. Here is additional message:

"Additional information: Attempted to read or write protected memory. This is often an indication that other memory is corrupt."

I am using the latest Chakra version with release build (x64).

@Havret
Copy link
Author

Havret commented May 24, 2016

Ok, I managed to resolve the problem. It seems when you are passing arguments to the function you have to call AddRef() on each JavaScriptValue, otherwise they could be collected before the function is actually called. After execution you can release them. I think it is quite important and it should be added to the documentation.

Here you have sample code snippets:

private static JavaScriptValue[] PrepareJavaScriptFunctionArguments(JavaScriptValue javaScriptThisArgument, object[] arguments)
{
    JavaScriptValue[] javaScriptFunctionArguments = new JavaScriptValue[arguments.Length + 1];

    javaScriptFunctionArguments[0] = javaScriptThisArgument;

    for (int i = 0; i < arguments.Length; i++)
    {
        javaScriptFunctionArguments[i + 1] = JavaScriptConverter.FromObject(arguments[i]);
        javaScriptFunctionArguments[i + 1].AddRef();

    }
    return javaScriptFunctionArguments;
}
private static object CallFunction(JavaScriptValue javaScriptFunction, JavaScriptValue[] javaScriptFunctionArguments)
{
    try
    {
        var result = javaScriptFunction.CallFunction(javaScriptFunctionArguments);
        var objectResult = JavaScriptConverter.ToObject(result);
        foreach (var argument in javaScriptFunctionArguments)
        {
            argument.Release();
        }
        return objectResult;
    }
    catch (JavaScriptScriptException exception)
    {
        throw new ChakraException(exception.Error);
    }
}

@liminzhu
Copy link
Collaborator

Thanks for the suggestion @Havret . You're pretty much right - you need to pin down JavaScriptValues with JsAddRef if these values are no longer on the stack. It's also important to release these objects after you're done so that GC can actually collect them, as you've shown in the second snippet. I'll look into adding documentation at a proper location and will post link here when I'm done.

@liminzhu
Copy link
Collaborator

Actually we already had it documented here. Closing the issue.

@mortenab
Copy link

Hi,

I know this issue has been closed, but I am having some concerns regarding the solution suggested by @Havret. Don't you still have a race condition with the garbage collector between the two following lines?

  javaScriptFunctionArguments[i + 1] = JavaScriptConverter.FromObject(arguments[i]);
  // garbage collection can happen here, collecting the newly created object
  javaScriptFunctionArguments[i + 1].AddRef();

Am I missing something here?
Thanks in advance,

Morten

@liminzhu
Copy link
Collaborator

@mortenab You still have a native stack reference to the JavaScriptValue s.t. GC won't collect the object.

@mortenab
Copy link

@liminzhu thanks for the reply.

So I guess the .NET stack is seen as part of the native stack?

If I for example implemented the function call in this way:

JavaScriptValue jsStr = JavaScriptValue.FromString("testArg");
var result = javaScriptFunction.CallFunction(new JavaScriptValue[] { JavaScriptValue.Null, jsStr });

I would not need to call AddRef on the jsStr, since it is on the stack?

Thanks,

Morten

@liminzhu
Copy link
Collaborator

No you're fine as long as the object is still on the C# stack.

@mortenab
Copy link

@liminzhu thanks for the clarification. I have created another issue in the samples repo: microsoft/Chakra-Samples#49
You are welcome to close that issue.

@dennis-yemelyanov
Copy link

@liminzhu sorry I have a couple more questions on this... I'm not sure I understand how C# call stack can prevent an object from being collected by JavaScript GC? I had an impression the two GCs are not really aware of each other?

Also does JavaScript GC always run continuously on a separate thread? Or does it pause while a host callback function is being executed?

@MSLaguana
Copy link
Contributor

The C# GC and the ChakraCore GC are not aware of each other, but the ChakraCore GC can walk the native stack and find pointers to things in the ChakraCore recycler and use those as roots. As long as the translation of various Js* handles into C# treats them as opaque and keeps the handle on the stack, then the ChakraCore recycler won't collect them.

The ChakraCore recycler doesn't run continuously (unless it has to due to app behavior), but it does do as much as possible away from the main thread. Most of the "marking" will happen on background threads, then the main thread will be stopped momentarily for a final pass to make sure that there is a consistent state of the world. At this point we also handle some cleanup synchronously, like invoking finalizers. Once that is done, most of the "sweep" of actually returning memory to the recycler to be used later happens on a background thread again.

@dennis-yemelyanov
Copy link

Thanks, @MSLaguana. That clarifies it a bit, however after more digging I found this discussion from which it sounds like it might not always be safe to rely on the compiler to actually keep values on the stack, and the recommendation seems to be to still call JsAddRef/JsRelease: #2836

Also I'm not sure if running a .NET host complicates matters even further compared to a native C host... I couldn't find much documentation on how .NET runtime interacts with the native stack and if it does any additional optimizations, but wanted to do more research on this.

Should I be worried about this at all? Or is it something that can happen in theory, but shouldn't really happen in practice?

@MSLaguana
Copy link
Contributor

I believe that the comments in that other issue were around a compiler noticing that a value doesn't seem to be live any more, and replacing it with something else. E.g.

JsValueRef foo = GetFoo();
JsDoAThing(foo);
// no further reference to foo in this function
int bar = 1;
// use bar

Even though foo is still in scope at the end there, a compiler is permitted to re-use the storage to hold bar since it knows that there are no further references to foo. This shouldn't be an issue in most cases, but if you're trying to be fancy with something like

JsValueRef foo = CreateFoo()
heapAllocatedStorage[0] = foo
// no further reference to foo
// bunch of code
JsDoAThing(heapAllocatedStorage[0])

then it's possible that between creating foo and re-using the same JsValueRef, even though foo is still in scope, it may have been evicted from the stack, and so a GC could have decided that there were no more references because the remaining references were hidden in the heap. But in the above case it would have been much more natural to call JsDoAThing(foo) which should have kept foo live the whole time and avoided the issue.

TL;DR: as long as you don't try to hide your references, and whenever you use heap allocated storage to hold a JsValueRef you invoke JsAddRef appropriately, then you should be fine.

@dennis-yemelyanov
Copy link

I see, that makes a lot of sense now. As always, thank you for your super promt response, detailed answers and patience while answering my questions :)

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

5 participants