microsoft / Chakra-Samples

Repository for Chakra JavaScript engine related samples.
MIT License
216 stars 84 forks source link

Details on .NET memory management requirements/gotchas #72

Open avonwyss opened 7 years ago

avonwyss commented 7 years ago

Based on lessons learned from hosting ChakraCore in a larger application, I'd have appreciated if the samples/docs had given some guidance on gotchas related to .NET memory management.

Specifically, I had hard to diagnose and understand problems with access violations, ChakraCore telling me that it was running on the wrong thread, etc. which all came from interop issues with callbacks.

Specifically, the .NET GC was collecting delegates which had been passed to ChakraCore (in my case through JsCreateFunction and JsSetPromiseContinuationCallback), which made it so that all unit tests were running fine but when running in the application with some memory pressure things started go awry.

I ended up adding a layer on top of Native.cs which creates and manages GCHandle (Normal mode, not pinned) instances for all delegates passed into ChakraCore APIs. When a context or a runtime is released, it will automatically release the associated GCHandles. With that all problems vanished...

liminzhu commented 7 years ago

Hi @avonwyss . This is the wiki detailing ChakraCore's memory management. I added a small para at the end for .NET.

Specifically, the .NET GC was collecting delegates which had been passed to ChakraCore (in my case through JsCreateFunction and JsSetPromiseContinuationCallback), which made it so that all unit tests were running fine but when running in the application with some memory pressure things started go awry.

The AV problem you mentioned is indeed because .NET GC is not aware of ChakraCore GC references. There were some discussions long ago about how to get around that.

ChakraCore telling me that it was running on the wrong thread

Is a different issue because of ChakraCore's rental threading design.

avonwyss commented 7 years ago

Hi @liminzhu

Thank you for the quick reaction. I think it would be best to somehow include this into the .NET samples, which is why I posted it here.

Regarding the wrong thread error, in my case it was really caused by the memory corruption; I had locks around every single access to JS context and the problem was intermittent with the AV errors. Also, it did not behave the same with and without debugger (got the wrong thread usually without debugger, and AV with debugger).

Note that pinning is not necessary and not advisable, keeping a Normal GCHandle is entierly sufficient. See also the GCHandleType docs.

liminzhu commented 7 years ago

I think it would be best to somehow include this into the .NET samples, which is why I posted it here.

Generally agree with your sentiment. Will try to come up with some short code snippet and documentation as said in https://github.com/Microsoft/ChakraCore-wiki/issues/5. Meanwhile, feel free to send a PR against Chakra-samples if you're interested in getting a sample online 😄.

Regarding the wrong thread error, in my case it was really caused by the memory corruption; I had locks around every single access to JS context and the problem was intermittent with the AV errors. Also, it did not behave the same with and without debugger (got the wrong thread usually without debugger, and AV with debugger).

Do you have a short repro I can play with? I would love to look more into it.

Note that pinning is not necessary and not advisable, keeping a Normal GCHandle is entierly sufficient. See also the GCHandleType docs.

Yep, I meant extending the lifetime at least till callback/object is no longer needed.