From bde1451f237cfd365ff1b29005eef9a8786bac19 Mon Sep 17 00:00:00 2001 From: Jason Ginchereau Date: Mon, 15 Jul 2024 20:57:15 -0700 Subject: [PATCH] JS Reference API improvements (#326) --- bench/Benchmarks.cs | 2 +- docs/features/js-references.md | 37 ++++---- src/NodeApi.DotNetHost/JSMarshaller.cs | 8 +- src/NodeApi.DotNetHost/ManagedHost.cs | 2 +- src/NodeApi.DotNetHost/NamespaceProxy.cs | 4 +- src/NodeApi.DotNetHost/TypeExporter.cs | 2 +- src/NodeApi/DotNetHost/NativeHost.cs | 2 +- src/NodeApi/Interop/JSAbortSignal.cs | 2 +- src/NodeApi/Interop/JSCollectionExtensions.cs | 4 +- src/NodeApi/Interop/JSInterface.cs | 2 +- src/NodeApi/Interop/JSRuntimeContext.cs | 8 +- src/NodeApi/Interop/NodeStream.Proxy.cs | 18 ++-- src/NodeApi/Interop/NodeStream.cs | 5 +- src/NodeApi/JSError.cs | 2 +- src/NodeApi/JSEventEmitter.cs | 24 +++--- src/NodeApi/JSReference.cs | 84 +++++++++++++++++-- src/NodeApi/JSTypedArray.cs | 3 +- src/NodeApi/Runtime/NodejsEnvironment.cs | 10 +-- test/JSReferenceTests.cs | 39 ++++++++- test/MockJSRuntime.cs | 11 +++ test/TestCases/node-addon-api/binding.cs | 2 +- 21 files changed, 197 insertions(+), 74 deletions(-) diff --git a/bench/Benchmarks.cs b/bench/Benchmarks.cs index 7f824964..d4b81f7d 100644 --- a/bench/Benchmarks.cs +++ b/bench/Benchmarks.cs @@ -217,7 +217,7 @@ public void CallDotnetMethodWithArgs() [Benchmark] public void ReferenceGet() { - _ = _reference.GetValue()!.Value; + _ = _reference.GetValue(); } [Benchmark] diff --git a/docs/features/js-references.md b/docs/features/js-references.md index 89e2ae19..3f8b8719 100644 --- a/docs/features/js-references.md +++ b/docs/features/js-references.md @@ -4,16 +4,13 @@ The [`JSReference`](../reference/dotnet/Microsoft.JavaScript.NodeApi/JSReference or weak reference to a JavaScript value. Use a reference to save a JS value on the .NET heap and enable it to be accessed later from a different [scope](./js-value-scopes). -::: warning -The example code below might need to be updated after -https://github.com/microsoft/node-api-dotnet/issues/197 is resolved. -::: - ## Using strong references A common practice is to save a reference as a member variable to support using the referenced JS value in a later callback. A strong reference prevents the JS value from being released -until the reference is disposed. +until the reference is disposed. The referenced value can be retrieved later via the +[`JSReference.GetValue()`](../reference/dotnet/Microsoft.JavaScript.NodeApi/JSReference/GetValue) +method. ```C# [JSExport] @@ -23,17 +20,17 @@ public class ReferenceExample : IDisposable public ReferenceExample(JSArray data) { - // The constructor must have been invoked from JS, or from .NET - // on the JS thread. Save a reference to the JS value parameter. + // The constructor must have been invoked from JS, or from .NET on the JS thread. + // Save a reference to the JS value parameter. DO NOT store the JSArray directly + // as a member because it will be invalid as soon as this method returns. _dataReference = new JSReference(data); } public double GetSum() { // Access the saved data value via the reference. - // Since the reference is strong, it never returns null. - // (It throws ObjectDisposedException if disposed.) - JSArray data = (JSArray)_dataReference.GetValue()!.Value; + // (It throws ObjectDisposedException if the reference is disposed.) + JSArray data = (JSArray)_dataReference.GetValue(); // JSArray implements IList. return data.Sum((JSValue value) => (double)value); @@ -50,12 +47,12 @@ public class ReferenceExample : IDisposable ## Using weak references -A weak reference does not prevent the JS value from being released. Therefore it is -necessary to check for null when getting the referenced value: +A weak reference does not prevent the JS value from being released. Use +[`JSReference.TryGetValue(out JSValue)`](../reference/dotnet/Microsoft.JavaScript.NodeApi/JSReference/TryGetValue) +to conditionally retrieve a weakly-referenced value if it is still available. ```C# -JSValue? value = reference.GetValue(); -if (value != null) +if (weakReference.TryGetValue(out JSValue value)) { // Do something with the value. } @@ -64,3 +61,13 @@ else // The JS value was released and is no longer available. } ``` + +## Reference limitations + +Currently only values of type `object`, `function`, and `symbol` can be referenced. It is not +possible to create a reference to other value types such as `string`, `number`, `boolean`, or +`undefined`. + +If the type of a value to be referenced is not known, use +[`JSReference.TryCreateReference()`](../reference/dotnet/Microsoft.JavaScript.NodeApi/JSReference/TryCreateReference) +and check the return value. diff --git a/src/NodeApi.DotNetHost/JSMarshaller.cs b/src/NodeApi.DotNetHost/JSMarshaller.cs index b4b6ab5d..9bb2a39f 100644 --- a/src/NodeApi.DotNetHost/JSMarshaller.cs +++ b/src/NodeApi.DotNetHost/JSMarshaller.cs @@ -892,7 +892,7 @@ private LambdaExpression BuildToJSFunctionExpression(Type delegateType) * var __valueRef = new JSReference(__value); * var __syncContext = JSSynchronizationContext.Current; * return (...args) => __syncContext.Run(() => - * __valueRef.GetValue().Value.Call(...args)); + * __valueRef.GetValue().Call(...args)); */ ParameterExpression valueParameter = Expression.Parameter(typeof(JSValue), "__value"); ParameterExpression valueRefVariable = Expression.Variable( @@ -912,11 +912,9 @@ private LambdaExpression BuildToJSFunctionExpression(Type delegateType) syncContextVariable, Expression.Property(null, typeof(JSSynchronizationContext).GetStaticProperty( nameof(JSSynchronizationContext.Current)))); - Expression getValueExpression = Expression.Property( - Expression.Call( + Expression getValueExpression = Expression.Call( valueRefVariable, - typeof(JSReference).GetInstanceMethod(nameof(JSReference.GetValue))), - "Value"); + typeof(JSReference).GetInstanceMethod(nameof(JSReference.GetValue))); ParameterExpression[] parameters = invokeMethod.GetParameters() .Select(Parameter).ToArray(); diff --git a/src/NodeApi.DotNetHost/ManagedHost.cs b/src/NodeApi.DotNetHost/ManagedHost.cs index 3b3aad9f..0ea172ae 100644 --- a/src/NodeApi.DotNetHost/ManagedHost.cs +++ b/src/NodeApi.DotNetHost/ManagedHost.cs @@ -345,7 +345,7 @@ public JSValue LoadModule(JSCallbackArgs args) if (_loadedModules.TryGetValue(assemblyFilePath, out JSReference? exportsRef)) { Trace("< ManagedHost.LoadModule() => already loaded"); - return exportsRef.GetValue()!.Value; + return exportsRef.GetValue(); } Assembly assembly; diff --git a/src/NodeApi.DotNetHost/NamespaceProxy.cs b/src/NodeApi.DotNetHost/NamespaceProxy.cs index 6258ecbe..201ab594 100644 --- a/src/NodeApi.DotNetHost/NamespaceProxy.cs +++ b/src/NodeApi.DotNetHost/NamespaceProxy.cs @@ -90,7 +90,7 @@ private JSProxy GetNamespaceObject() { if (_valueReference != null) { - return (JSProxy)_valueReference.GetValue()!.Value; + return (JSProxy)_valueReference.GetValue(); } JSProxy proxy = new(new JSObject(), CreateProxyHandler()); @@ -102,7 +102,7 @@ private JSFunction GetToStringFunction() { if (_tostringReference != null) { - return (JSFunction)_tostringReference.GetValue()!.Value; + return (JSFunction)_tostringReference.GetValue(); } // Calling `toString()` on a namespace returns the full namespace name. diff --git a/src/NodeApi.DotNetHost/TypeExporter.cs b/src/NodeApi.DotNetHost/TypeExporter.cs index 34c04f13..5d7e31ba 100644 --- a/src/NodeApi.DotNetHost/TypeExporter.cs +++ b/src/NodeApi.DotNetHost/TypeExporter.cs @@ -125,7 +125,7 @@ public void ExportAssemblyTypes(Assembly assembly) if (_namespaces != null) { // Add a property on the namespaces JS object. - JSObject namespacesObject = (JSObject)_namespaces.GetValue()!.Value; + JSObject namespacesObject = (JSObject)_namespaces.GetValue(); namespacesObject[namespaceParts[0]] = parentNamespace.Value; } } diff --git a/src/NodeApi/DotNetHost/NativeHost.cs b/src/NodeApi/DotNetHost/NativeHost.cs index b5a52ecc..c71bcaf5 100644 --- a/src/NodeApi/DotNetHost/NativeHost.cs +++ b/src/NodeApi/DotNetHost/NativeHost.cs @@ -105,7 +105,7 @@ private JSValue InitializeManagedHost(JSCallbackArgs args) // Normally this shouldn't happen because the host package initialization // script would only be loaded once by require(). But certain situations like // drive letter or path casing inconsistencies can cause it to be loaded twice. - return _exports.GetValue()!.Value; + return _exports.GetValue(); } else { diff --git a/src/NodeApi/Interop/JSAbortSignal.cs b/src/NodeApi/Interop/JSAbortSignal.cs index 7074ceca..c072b51d 100644 --- a/src/NodeApi/Interop/JSAbortSignal.cs +++ b/src/NodeApi/Interop/JSAbortSignal.cs @@ -90,7 +90,7 @@ private static JSAbortSignal FromCancellationToken(CancellationToken cancellatio JSSynchronizationContext syncContext = JSSynchronizationContext.Current!; cancellation.Register(() => syncContext.Post(() => { - controllerReference.GetValue()!.Value.CallMethod("abort"); + controllerReference.GetValue().CallMethod("abort"); controllerReference.Dispose(); })); return new JSAbortSignal(controller["signal"]); diff --git a/src/NodeApi/Interop/JSCollectionExtensions.cs b/src/NodeApi/Interop/JSCollectionExtensions.cs index feac8a2d..1b9a1012 100644 --- a/src/NodeApi/Interop/JSCollectionExtensions.cs +++ b/src/NodeApi/Interop/JSCollectionExtensions.cs @@ -367,7 +367,7 @@ internal JSIterableEnumerable(JSValue iterable, JSValue.To fromJS) private readonly JSReference _iterableReference; - internal JSValue Value => _iterableReference.GetValue()!.Value; + internal JSValue Value => _iterableReference.GetValue(); protected void Run(Action action) => _iterableReference.Run(action); protected TResult Run(Func action) => _iterableReference.Run(action); @@ -664,7 +664,7 @@ internal JSMapReadOnlyDictionary( protected void Run(Action action) => _mapReference.Run(action); protected TResult Run(Func action) => _mapReference.Run(action); - internal JSValue Value => _mapReference.GetValue()!.Value; + internal JSValue Value => _mapReference.GetValue(); bool IEquatable.Equals(JSValue other) => Run((map) => map.Equals(other)); diff --git a/src/NodeApi/Interop/JSInterface.cs b/src/NodeApi/Interop/JSInterface.cs index 7b07c0d2..c476e8a3 100644 --- a/src/NodeApi/Interop/JSInterface.cs +++ b/src/NodeApi/Interop/JSInterface.cs @@ -31,7 +31,7 @@ protected JSInterface(JSValue value) /// /// Gets the underlying JS value. /// - protected JSValue Value => ValueReference.GetValue()!.Value; + protected JSValue Value => ValueReference.GetValue(); /// /// Dynamically invokes an interface method JS adapter delegate after obtaining the JS `this` diff --git a/src/NodeApi/Interop/JSRuntimeContext.cs b/src/NodeApi/Interop/JSRuntimeContext.cs index 9492608c..1fe533a8 100644 --- a/src/NodeApi/Interop/JSRuntimeContext.cs +++ b/src/NodeApi/Interop/JSRuntimeContext.cs @@ -311,7 +311,7 @@ internal JSValue InitializeObjectWrapper(JSValue wrapper, JSValue externalInstan if (_objectMap.TryGetValue(obj, out JSReference? existingWrapperWeakRef)) { - if (!existingWrapperWeakRef.IsDisposed && existingWrapperWeakRef.GetValue().HasValue) + if (!existingWrapperWeakRef.IsDisposed && existingWrapperWeakRef.TryGetValue(out _)) { // If the .NET object is already mapped to a non-released JS object, then // another one should not be created. @@ -626,6 +626,9 @@ public JSValue Import( JSReference reference = _importMap.GetOrAdd((module, property), (_) => { + // TODO: Consider what to do if the imported property is undefined + // or not a referenceable type. + if (module == null || module == "global") { // Importing a built-in object from `global`. @@ -656,9 +659,8 @@ public JSValue Import( return new JSReference(propertyValue); } } - }); - return reference.GetValue() ?? JSValue.Undefined; + return reference.GetValue(); } /// diff --git a/src/NodeApi/Interop/NodeStream.Proxy.cs b/src/NodeApi/Interop/NodeStream.Proxy.cs index 60346710..9cdefc4d 100644 --- a/src/NodeApi/Interop/NodeStream.Proxy.cs +++ b/src/NodeApi/Interop/NodeStream.Proxy.cs @@ -97,7 +97,7 @@ private static JSValue GetOrCreateAdapter(bool readable, bool writable) { if (s_duplexStreamAdapterReference != null) { - return s_duplexStreamAdapterReference.GetValue()!.Value; + return s_duplexStreamAdapterReference.GetValue(); } var streamAdapter = new JSObject @@ -116,7 +116,7 @@ private static JSValue GetOrCreateAdapter(bool readable, bool writable) { if (s_readableStreamAdapterReference != null) { - return s_readableStreamAdapterReference.GetValue()!.Value; + return s_readableStreamAdapterReference.GetValue(); } var streamAdapter = new JSObject @@ -132,7 +132,7 @@ private static JSValue GetOrCreateAdapter(bool readable, bool writable) { if (s_writableStreamAdapterReference != null) { - return s_writableStreamAdapterReference.GetValue()!.Value; + return s_writableStreamAdapterReference.GetValue(); } var streamAdapter = new JSObject @@ -181,7 +181,7 @@ private static async void ReadAsync( int count = await stream.ReadAsync(buffer, 0, ReadChunkSize); #endif - nodeStream = nodeStreamReference.GetValue()!.Value; + nodeStream = nodeStreamReference.GetValue(); nodeStream.CallMethod( "push", count == 0 ? JSValue.Null : new JSTypedArray(buffer, 0, count)); } @@ -189,7 +189,7 @@ private static async void ReadAsync( { try { - nodeStream = nodeStreamReference.GetValue()!.Value; + nodeStream = nodeStreamReference.GetValue(); nodeStream.CallMethod("destroy", new JSError(ex).Value); } catch (Exception) @@ -253,7 +253,7 @@ private static async void WriteAsync( await stream.WriteAsync(chunk.ToArray(), 0, chunk.Length); #endif - callback = callbackReference.GetValue()!.Value; + callback = callbackReference.GetValue(); callback.Call(); } catch (Exception ex) @@ -261,7 +261,7 @@ private static async void WriteAsync( bool isExceptionPending5 = JSError.IsExceptionPending(); try { - callback = callbackReference.GetValue()!.Value; + callback = callbackReference.GetValue(); callback.Call(thisArg: JSValue.Undefined, new JSError(ex).Value); } catch (Exception) @@ -292,14 +292,14 @@ private static async void FinalAsync( { await stream.FlushAsync(); - callback = callbackReference.GetValue()!.Value; + callback = callbackReference.GetValue(); callback.Call(); } catch (Exception ex) { try { - callback = callbackReference.GetValue()!.Value; + callback = callbackReference.GetValue(); callback.Call(thisArg: JSValue.Undefined, new JSError(ex).Value); } catch (Exception) diff --git a/src/NodeApi/Interop/NodeStream.cs b/src/NodeApi/Interop/NodeStream.cs index 32a042fb..0b1d0307 100644 --- a/src/NodeApi/Interop/NodeStream.cs +++ b/src/NodeApi/Interop/NodeStream.cs @@ -18,7 +18,7 @@ public partial class NodeStream : Stream public static explicit operator NodeStream(JSValue value) => new(value); public static implicit operator JSValue(NodeStream stream) - => stream._valueReference.GetValue() ?? default; + => stream._valueReference.GetValue(); private NodeStream(JSValue value) { @@ -53,8 +53,7 @@ private NodeStream(JSValue value) })); } - private JSValue Value => _valueReference.GetValue() ?? - throw new ObjectDisposedException(nameof(NodeStream)); + private JSValue Value => _valueReference.GetValue(); public override bool CanRead => Value.HasProperty("read"); diff --git a/src/NodeApi/JSError.cs b/src/NodeApi/JSError.cs index 822c6d40..ff875cf2 100644 --- a/src/NodeApi/JSError.cs +++ b/src/NodeApi/JSError.cs @@ -153,7 +153,7 @@ public string Message { try { - _message = (string?)_errorRef.GetValue()?["message"]; + _message = (string?)_errorRef.GetValue()["message"]; } catch { diff --git a/src/NodeApi/JSEventEmitter.cs b/src/NodeApi/JSEventEmitter.cs index 663c6548..e94847b1 100644 --- a/src/NodeApi/JSEventEmitter.cs +++ b/src/NodeApi/JSEventEmitter.cs @@ -41,7 +41,7 @@ public void AddListener(string eventName, JSValue listener) { if (_nodeEmitter != null) { - _nodeEmitter.GetValue()!.Value.CallMethod("addListener", eventName, listener); + _nodeEmitter.GetValue().CallMethod("addListener", eventName, listener); return; } @@ -53,7 +53,7 @@ public void AddListener(string eventName, JSValue listener) JSArray eventListeners; if (_listeners!.TryGetValue(eventName, out JSReference? eventListenersReference)) { - eventListeners = (JSArray)eventListenersReference.GetValue()!.Value; + eventListeners = (JSArray)eventListenersReference.GetValue(); } else { @@ -68,13 +68,13 @@ public void RemoveListener(string eventName, JSValue listener) { if (_nodeEmitter != null) { - _nodeEmitter.GetValue()!.Value.CallMethod("removeListener", eventName, listener); + _nodeEmitter.GetValue().CallMethod("removeListener", eventName, listener); return; } if (_listeners!.TryGetValue(eventName, out JSReference? eventListenersReference)) { - JSArray eventListeners = (JSArray)eventListenersReference.GetValue()!.Value; + JSArray eventListeners = (JSArray)eventListenersReference.GetValue(); eventListeners.Remove(listener); } } @@ -83,7 +83,7 @@ public void Once(string eventName, JSCallback listener) { if (_nodeEmitter != null) { - _nodeEmitter.GetValue()!.Value.CallMethod( + _nodeEmitter.GetValue().CallMethod( "once", eventName, JSValue.CreateFunction(eventName, listener)); return; } @@ -103,7 +103,7 @@ public void Once(string eventName, JSValue listener) { if (_nodeEmitter != null) { - _nodeEmitter.GetValue()!.Value.CallMethod("once", eventName, listener); + _nodeEmitter.GetValue().CallMethod("once", eventName, listener); return; } @@ -139,13 +139,13 @@ public void Emit(string eventName) { if (_nodeEmitter != null) { - _nodeEmitter.GetValue()!.Value.CallMethod("emit", eventName); + _nodeEmitter.GetValue().CallMethod("emit", eventName); return; } if (_listeners!.TryGetValue(eventName, out JSReference? eventListenersReference)) { - JSArray eventListeners = (JSArray)eventListenersReference.GetValue()!.Value; + JSArray eventListeners = (JSArray)eventListenersReference.GetValue(); foreach (JSValue listener in eventListeners) { listener.Call(thisArg: default); @@ -157,13 +157,13 @@ public void Emit(string eventName, JSValue arg) { if (_nodeEmitter != null) { - _nodeEmitter.GetValue()!.Value.CallMethod("emit", eventName, arg); + _nodeEmitter.GetValue().CallMethod("emit", eventName, arg); return; } if (_listeners!.TryGetValue(eventName, out JSReference? eventListenersReference)) { - JSArray eventListeners = (JSArray)eventListenersReference.GetValue()!.Value; + JSArray eventListeners = (JSArray)eventListenersReference.GetValue(); foreach (JSValue listener in eventListeners) { listener.Call(thisArg: default, arg); @@ -178,13 +178,13 @@ public void Emit(string eventName, params JSValue[] args) JSValue[] argsArray = new JSValue[args.Length + 1]; argsArray[0] = eventName; args.CopyTo(argsArray, 1); - _nodeEmitter.GetValue()!.Value.CallMethod("emit", argsArray); + _nodeEmitter.GetValue().CallMethod("emit", argsArray); return; } if (_listeners!.TryGetValue(eventName, out JSReference? eventListenersReference)) { - JSArray eventListeners = (JSArray)eventListenersReference.GetValue()!.Value; + JSArray eventListeners = (JSArray)eventListenersReference.GetValue(); foreach (JSValue listener in eventListeners) { listener.Call(thisArg: default, args); diff --git a/src/NodeApi/JSReference.cs b/src/NodeApi/JSReference.cs index c5667fb3..2bf0cccb 100644 --- a/src/NodeApi/JSReference.cs +++ b/src/NodeApi/JSReference.cs @@ -18,9 +18,13 @@ namespace Microsoft.JavaScript.NodeApi; /// to maintain a reference to a JS value beyond a single scope. /// /// A should be disposed when no longer needed; this allows the JS value -/// to be collected by the GC if it has no other references. The class +/// to be collected by the JS GC if it has no other references. The class /// also has a finalizer so that the reference will be released when the C# object is GC'd. However /// explicit disposal is still preferable when possible. +/// +/// A "strong" reference ensures the JS value is available at least until the reference is disposed. +/// A "weak" reference does not prevent the JS value from being released collected by the JS +/// garbage-collector, but allows the value to be optimistically retrieved until then. /// public class JSReference : IDisposable { @@ -28,8 +32,16 @@ public class JSReference : IDisposable private readonly napi_env _env; private readonly JSRuntimeContext? _context; - public bool IsWeak { get; private set; } - + /// + /// Creates a new instance of a that holds a strong or weak + /// reference to a JS value. + /// + /// The JavaScript value to reference. + /// True if the reference will be "weak", meaning the reference does not + /// prevent the value from being released and collected by the JS garbage-collector. The + /// default is false, meaning the value will remain available at least until the "strong" + /// reference is disposed. + /// public JSReference(JSValue value, bool isWeak = false) : this(value.Runtime.CreateReference( (napi_env)JSValueScope.Current, @@ -40,6 +52,13 @@ public JSReference(JSValue value, bool isWeak = false) { } + /// + /// Creates a new instance of a that holds a strong or weak + /// reference to a JS value. + /// + /// The reference handle. + /// True if the handle is for a weak reference. This must match + /// the existing state of the handle. public JSReference(napi_ref handle, bool isWeak = false) { JSValueScope currentScope = JSValueScope.Current; @@ -51,6 +70,12 @@ public JSReference(napi_ref handle, bool isWeak = false) IsWeak = isWeak; } + /// + /// Gets a value indicating whether the reference is weak. + /// + /// True if the reference is weak, false if it is strong. + public bool IsWeak { get; private set; } + /// /// Gets the value handle, or throws an exception if access from the current thread is invalid. /// @@ -73,6 +98,16 @@ public static explicit operator napi_ref(JSReference reference) return reference.Handle; } + /// + /// Creates a reference to a JS value, if the value is an object, function, or symbol. + /// + /// The JS value to reference. + /// True to create a weak reference, false to create a strong + /// reference. + /// Returns the created reference, or default(JSValue), equivalent + /// to undefined, if the value is not a referencable type. + /// True if the reference was created, false if the reference could not be created + /// because the value is not a supported type for references. public static bool TryCreateReference( JSValue value, bool isWeak, [NotNullWhen(true)] out JSReference? result) { @@ -139,17 +174,54 @@ public void MakeStrong() } /// - /// Gets the referenced JS value, or null if the reference is weak and the value is - /// no longer available. + /// Gets the referenced JS value. /// + /// The referenced JS value. /// The reference is disposed. - public JSValue? GetValue() + /// The reference is weak and the weakly-referenced + /// JS value is not available. + /// + /// Use this method with strong references when the referenced value is expected to be always + /// available. For weak references, use instead. + /// + public JSValue GetValue() { JSValueScope.CurrentRuntime.GetReferenceValue(Env, _handle, out napi_value result) .ThrowIfFailed(); + + // napi_get_reference_value() returns a null handle if the weak reference is invalid. + if (result == default) + { + throw new NullReferenceException("The weakly-referenced JS value not available."); + } + return result; } + /// + /// Attempts to get the referenced JS value. + /// + /// Returns the referenced JS value, or default(JSValue), equivalent + /// to undefined, if the reference is weak and the value is not available. + /// True if the value was obtained, false if the reference is weak and the value is + /// not available. + /// The reference is disposed. + public bool TryGetValue(out JSValue value) + { + JSValueScope.CurrentRuntime.GetReferenceValue(Env, _handle, out napi_value result) + .ThrowIfFailed(); + + // napi_get_reference_value() returns a null handle if the weak reference is invalid. + if (result == default) + { + value = default; + return false; + } + + value = new JSValue(result); + return true; + } + /// /// Runs an action with the referenced value, using the /// associated with the reference to switch to the JS thread (if necessary) while operating diff --git a/src/NodeApi/JSTypedArray.cs b/src/NodeApi/JSTypedArray.cs index e961da9f..16324186 100644 --- a/src/NodeApi/JSTypedArray.cs +++ b/src/NodeApi/JSTypedArray.cs @@ -217,8 +217,7 @@ public MemoryManager(JSTypedArray typedArray) _typedArrayReference = new JSReference(typedArray); } - public JSValue JSValue => _typedArrayReference.GetValue() ?? - throw new ObjectDisposedException(nameof(JSTypedArray)); + public JSValue JSValue => _typedArrayReference.GetValue(); public override Span GetSpan() { diff --git a/src/NodeApi/Runtime/NodejsEnvironment.cs b/src/NodeApi/Runtime/NodejsEnvironment.cs index 1f728e9e..1debc2fb 100644 --- a/src/NodeApi/Runtime/NodejsEnvironment.cs +++ b/src/NodeApi/Runtime/NodejsEnvironment.cs @@ -90,7 +90,7 @@ private static void InitializeModuleImportFunctions( JSReference originalRequireRef = new(originalRequire); JSFunction envRequire = new("require", (modulePath) => { - JSValue require = originalRequireRef.GetValue()!.Value; + JSValue require = originalRequireRef.GetValue(); JSValue resolvedPath = ResolveModulePath(require, modulePath, baseDir); return require.Call(thisArg: default, resolvedPath); }); @@ -99,7 +99,7 @@ private static void InitializeModuleImportFunctions( JSValue requireObject = (JSValue)envRequire; requireObject["resolve"] = new JSFunction("resolve", (modulePath) => { - JSValue require = originalRequireRef.GetValue()!.Value; + JSValue require = originalRequireRef.GetValue(); return ResolveModulePath(require, modulePath, baseDir); }); @@ -126,10 +126,10 @@ private static void InitializeModuleImportFunctions( JSReference originalImportRef = new(originalImport); JSFunction envImport = new("import", (modulePath) => { - JSValue require = originalRequireRef.GetValue()!.Value; + JSValue require = originalRequireRef.GetValue(); JSValue resolvedPath = ResolveModulePath(require, modulePath, baseDir); JSValue moduleUri = "file://" + (string)resolvedPath; - JSValue import = originalImportRef.GetValue()!.Value; + JSValue import = originalImportRef.GetValue(); return import.Call(thisArg: default, moduleUri); }); @@ -286,7 +286,7 @@ private void AddUnhandledPromiseRejectionListener() private void RemoveUnhandledPromiseRejectionListener() { - JSValue listener = _unhandledPromiseRejectionListener!.GetValue()!.Value; + JSValue listener = _unhandledPromiseRejectionListener!.GetValue(); JSValue.Global["process"].CallMethod("off", "unhandledRejection", listener); _unhandledPromiseRejectionListener.Dispose(); _unhandledPromiseRejectionListener = null; diff --git a/test/JSReferenceTests.cs b/test/JSReferenceTests.cs index db8bc5aa..1283cbec 100644 --- a/test/JSReferenceTests.cs +++ b/test/JSReferenceTests.cs @@ -25,7 +25,7 @@ public void GetReferenceFromSameScope() JSValue value = JSValue.CreateObject(); JSReference reference = new(value); - Assert.True(reference.GetValue()?.IsObject() ?? false); + Assert.True(reference.GetValue().IsObject()); } [Fact] @@ -40,7 +40,7 @@ public void GetReferenceFromParentScope() reference = new JSReference(value); } - Assert.True(reference.GetValue()?.IsObject() ?? false); + Assert.True(reference.GetValue().IsObject()); } [Fact] @@ -73,4 +73,39 @@ public void GetReferenceFromDifferentRootScope() Assert.Throws(() => reference.GetValue()); }).Wait(); } + + [Fact] + public void GetWeakReferenceUnavailable() + { + using JSValueScope rootScope = TestScope(JSValueScopeType.Root); + + JSValue value = JSValue.CreateObject(); + var reference = new JSReference(value, isWeak: true); + + _mockRuntime.MockReleaseWeakReferenceValue(reference.Handle); + Assert.Throws(() => reference.GetValue()); + } + + [Fact] + public void TryGetWeakReferenceValue() + { + using JSValueScope rootScope = TestScope(JSValueScopeType.Root); + + JSValue value = JSValue.CreateObject(); + JSReference reference = new(value); + Assert.True(reference.TryGetValue(out JSValue result)); + Assert.True(result.IsObject()); + } + + [Fact] + public void TryGetWeakReferenceUnavailable() + { + using JSValueScope rootScope = TestScope(JSValueScopeType.Root); + + JSValue value = JSValue.CreateObject(); + var reference = new JSReference(value, isWeak: true); + + _mockRuntime.MockReleaseWeakReferenceValue(reference.Handle); + Assert.False(reference.TryGetValue(out _)); + } } diff --git a/test/MockJSRuntime.cs b/test/MockJSRuntime.cs index f4f738bc..ccd923bc 100644 --- a/test/MockJSRuntime.cs +++ b/test/MockJSRuntime.cs @@ -187,6 +187,17 @@ public override napi_status DeleteReference(napi_env env, napi_ref @ref) return _references.Remove(@ref.Handle) ? napi_ok : napi_invalid_arg; } + /// + /// Simulates the behavior of the JS runtime when a weakly-referenced value is released. + /// + public void MockReleaseWeakReferenceValue(napi_ref @ref) + { + if (_references.TryGetValue(@ref.Handle, out MockJSRef? mockRef)) + { + mockRef.ValueHandle = 0; + } + } + // Mocking the sync context prevents the runtime mock from having to implement APIs // to support initializing the thread-safe-function for the sync context. // Unit tests that use the mock runtime don't currently use the sync context. diff --git a/test/TestCases/node-addon-api/binding.cs b/test/TestCases/node-addon-api/binding.cs index c6b116df..38ce9228 100644 --- a/test/TestCases/node-addon-api/binding.cs +++ b/test/TestCases/node-addon-api/binding.cs @@ -26,7 +26,7 @@ public class Binding { if (_testObjects.TryGetValue(typeof(T), out JSReference? testRef)) { - return testRef.GetValue() ?? JSValue.Undefined; + return testRef.GetValue(); } JSValue obj = new T().Init();