microsoft / node-api-dotnet

Advanced interoperability between .NET and JavaScript in the same process.
MIT License
426 stars 49 forks source link

Crash in JSValue.Unwrap() after repeated calls to .NET object async method #296

Closed jasongin closed 1 month ago

jasongin commented 1 month ago

Certain call patterns from JS to .NET can cause a crash when "unwrapping" the this object for a method call, in JSValue.Unwrap() on this line:

return GCHandle.FromIntPtr(result).Target!;

The crash is due to a null or invalid handle (result) value. It seems to be more likely when calling .NET async methods from JS. And much easier to repro by forcing a .NET and/or Node.js GC.

Repro steps:

  1. Check out the repro branch dev/jasongin/gchandle
  2. dotnet pack from root (for some extra debugging output related to GCHandle wrap/unwrap)
  3. cd examples/dotnet-module
  4. dotnet build
  5. node --expose-gc example.js

Occasionally it will run and just keep counting with no errors. But usually it crashes right away with an exception similar to this:

Unwrapping GC handle 0000000000000004 Error unwrapping 0000000000000004: System.NullReferenceException: Object reference not set to an instance of an object. at System.Runtime.InteropServices.GCHandle.get_Target() at Microsoft.JavaScript.NodeApi.JSValue.Unwrap(String unwrapType)

Or sometimes the GC handle value might be more random junk that causes an AV:

Unwrapping handle 00007FFAC2233D88 Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication > that other memory is corrupt.

In either case the GC handle value is wrong, since the correct handle values can be seen in previous logging. For some reason napi_unwrap() is returning bad values, even when its return value is napi_ok. It is definitely being called from the JS thread, since it is the start of a callback, unwrapping the this value.