Skip to content

Commit 30cd409

Browse files
committed
Address PR feedback
1 parent 3d50ced commit 30cd409

File tree

8 files changed

+54
-55
lines changed

8 files changed

+54
-55
lines changed

src/NodeApi/Runtime/NodeEmbeddingNodeApiScope.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@ public sealed class NodeEmbeddingNodeApiScope : IDisposable
1616
public NodeEmbeddingNodeApiScope(NodeEmbeddingRuntime runtime)
1717
{
1818
_runtime = runtime;
19-
NodeEmbeddingRuntime.JSRuntime.EmbeddingRuntimeOpenNodeApiScope(
19+
NodeEmbedding.JSRuntime.EmbeddingRuntimeOpenNodeApiScope(
2020
runtime.Handle, out _nodeApiScope, out napi_env env)
2121
.ThrowIfFailed();
2222
_valueScope = new JSValueScope(
23-
JSValueScopeType.Root, env, NodeEmbeddingRuntime.JSRuntime);
23+
JSValueScopeType.Root, env, NodeEmbedding.JSRuntime);
2424
}
2525

2626
/// <summary>
@@ -37,7 +37,7 @@ public void Dispose()
3737
IsDisposed = true;
3838

3939
_valueScope.Dispose();
40-
NodeEmbeddingRuntime.JSRuntime.EmbeddingRuntimeCloseNodeApiScope(
40+
NodeEmbedding.JSRuntime.EmbeddingRuntimeCloseNodeApiScope(
4141
_runtime.Handle, _nodeApiScope)
4242
.ThrowIfFailed();
4343
}

src/NodeApi/Runtime/NodeEmbeddingPlatform.cs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public NodeEmbeddingPlatform(string libNodePath, NodeEmbeddingPlatformSettings?
4141

4242
using FunctorRef<node_embedding_platform_configure_callback> functorRef =
4343
CreatePlatformConfigureFunctorRef(settings?.CreateConfigurePlatformCallback());
44-
JSRuntime.EmbeddingCreatePlatform(
44+
NodeEmbedding.JSRuntime.EmbeddingCreatePlatform(
4545
settings?.Args ?? new string[] { "node" },
4646
functorRef.Callback,
4747
functorRef.Data,
@@ -61,8 +61,6 @@ public NodeEmbeddingPlatform(string libNodePath, NodeEmbeddingPlatformSettings?
6161
/// </summary>
6262
public static NodeEmbeddingPlatform? Current { get; private set; }
6363

64-
public static JSRuntime JSRuntime => NodeEmbedding.JSRuntime;
65-
6664
/// <summary>
6765
/// Disposes the platform. After disposal, another platform instance may not be initialized
6866
/// in the current process.
@@ -71,7 +69,7 @@ public void Dispose()
7169
{
7270
if (IsDisposed) return;
7371
IsDisposed = true;
74-
JSRuntime.EmbeddingDeletePlatform(_platform);
72+
NodeEmbedding.JSRuntime.EmbeddingDeletePlatform(_platform);
7573
}
7674

7775
/// <summary>
@@ -98,7 +96,7 @@ public unsafe string[] GetParsedArgs()
9896

9997
int argc = 0;
10098
nint argv = 0;
101-
JSRuntime.EmbeddingPlatformGetParsedArgs(
99+
NodeEmbedding.JSRuntime.EmbeddingPlatformGetParsedArgs(
102100
_platform, (nint)(&argc), (nint)(&argv), 0, 0).ThrowIfFailed();
103101
return Utf8StringArray.ToStringArray(argv, argc);
104102
}
@@ -109,7 +107,7 @@ public unsafe string[] GetRuntimeParsedArgs()
109107

110108
int argc = 0;
111109
nint argv = 0;
112-
JSRuntime.EmbeddingPlatformGetParsedArgs(
110+
NodeEmbedding.JSRuntime.EmbeddingPlatformGetParsedArgs(
113111
_platform, 0, 0, (nint)(&argc), (nint)(&argv)).ThrowIfFailed();
114112
return Utf8StringArray.ToStringArray(argv, argc);
115113
}

src/NodeApi/Runtime/NodeEmbeddingPlatformSettings.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,12 @@ public class NodeEmbeddingPlatformSettings
1212
public string[]? Args { get; set; }
1313
public ConfigurePlatformCallback? ConfigurePlatform { get; set; }
1414

15-
public static JSRuntime JSRuntime => NodeEmbedding.JSRuntime;
16-
1715
public unsafe ConfigurePlatformCallback CreateConfigurePlatformCallback()
1816
=> new((config) =>
1917
{
2018
if (PlatformFlags != null)
2119
{
22-
JSRuntime.EmbeddingPlatformConfigSetFlags(config, PlatformFlags.Value)
20+
NodeEmbedding.JSRuntime.EmbeddingPlatformConfigSetFlags(config, PlatformFlags.Value)
2321
.ThrowIfFailed();
2422
}
2523
ConfigurePlatform?.Invoke(config);

src/NodeApi/Runtime/NodeEmbeddingRuntime.cs

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,12 @@ public sealed class NodeEmbeddingRuntime : IDisposable
2121
{
2222
private bool _mustDeleteRuntime; // Only delete runtime if it was created by calling Create.
2323

24-
public static JSRuntime JSRuntime => NodeEmbedding.JSRuntime;
25-
2624
public static unsafe NodeEmbeddingRuntime Create(
2725
NodeEmbeddingPlatform platform, NodeEmbeddingRuntimeSettings? settings = null)
2826
{
2927
using FunctorRef<node_embedding_runtime_configure_callback> functorRef =
3028
CreateRuntimeConfigureFunctorRef(settings?.CreateConfigureRuntimeCallback());
31-
JSRuntime.EmbeddingCreateRuntime(
29+
NodeEmbedding.JSRuntime.EmbeddingCreateRuntime(
3230
platform.Handle,
3331
functorRef.Callback,
3432
functorRef.Data,
@@ -48,14 +46,15 @@ private NodeEmbeddingRuntime(node_embedding_runtime runtime)
4846

4947
public static unsafe NodeEmbeddingRuntime FromHandle(node_embedding_runtime runtime)
5048
{
51-
JSRuntime.EmbeddingRuntimeGetUserData(runtime, out nint userData).ThrowIfFailed();
49+
NodeEmbedding.JSRuntime.EmbeddingRuntimeGetUserData(runtime, out nint userData)
50+
.ThrowIfFailed();
5251
if (userData != default)
5352
{
5453
return (NodeEmbeddingRuntime)GCHandle.FromIntPtr(userData).Target!;
5554
}
5655

5756
NodeEmbeddingRuntime result = new(runtime);
58-
JSRuntime.EmbeddingRuntimeSetUserData(
57+
NodeEmbedding.JSRuntime.EmbeddingRuntimeSetUserData(
5958
runtime,
6059
(nint)GCHandle.Alloc(result),
6160
new node_embedding_data_release_callback(s_releaseRuntimeCallback))
@@ -68,7 +67,8 @@ public static unsafe void Run(
6867
{
6968
using FunctorRef<node_embedding_runtime_configure_callback> functorRef =
7069
CreateRuntimeConfigureFunctorRef(settings?.CreateConfigureRuntimeCallback());
71-
JSRuntime.EmbeddingRunRuntime(platform.Handle, functorRef.Callback, functorRef.Data)
70+
NodeEmbedding.JSRuntime.EmbeddingRunRuntime(
71+
platform.Handle, functorRef.Callback, functorRef.Data)
7272
.ThrowIfFailed();
7373
}
7474

@@ -85,36 +85,38 @@ public void Dispose()
8585
if (IsDisposed || !_mustDeleteRuntime) return;
8686
IsDisposed = true;
8787

88-
JSRuntime.EmbeddingDeleteRuntime(Handle).ThrowIfFailed();
88+
NodeEmbedding.JSRuntime.EmbeddingDeleteRuntime(Handle).ThrowIfFailed();
8989
}
9090

9191
public unsafe void RunEventLoop()
9292
{
9393
if (IsDisposed) throw new ObjectDisposedException(nameof(NodeEmbeddingRuntime));
9494

95-
JSRuntime.EmbeddingRuntimeRunEventLoop(Handle).ThrowIfFailed();
95+
NodeEmbedding.JSRuntime.EmbeddingRuntimeRunEventLoop(Handle).ThrowIfFailed();
9696
}
9797

9898
public unsafe void TerminateEventLoop()
9999
{
100100
if (IsDisposed) throw new ObjectDisposedException(nameof(NodeEmbeddingRuntime));
101101

102-
JSRuntime.EmbeddingRuntimeTerminateEventLoop(Handle).ThrowIfFailed();
102+
NodeEmbedding.JSRuntime.EmbeddingRuntimeTerminateEventLoop(Handle).ThrowIfFailed();
103103
}
104104

105105
public unsafe bool RunEventLoopOnce()
106106
{
107107
if (IsDisposed) throw new ObjectDisposedException(nameof(NodeEmbeddingRuntime));
108108

109-
JSRuntime.EmbeddingRuntimeRunOnceEventLoop(Handle, out bool result).ThrowIfFailed();
109+
NodeEmbedding.JSRuntime.EmbeddingRuntimeRunOnceEventLoop(Handle, out bool result)
110+
.ThrowIfFailed();
110111
return result;
111112
}
112113

113114
public unsafe bool RunEventLoopNoWait()
114115
{
115116
if (IsDisposed) throw new ObjectDisposedException(nameof(NodeEmbeddingRuntime));
116117

117-
JSRuntime.EmbeddingRuntimeRunNoWaitEventLoop(Handle, out bool result).ThrowIfFailed();
118+
NodeEmbedding.JSRuntime.EmbeddingRuntimeRunNoWaitEventLoop(Handle, out bool result)
119+
.ThrowIfFailed();
118120
return result;
119121
}
120122

@@ -124,7 +126,8 @@ public unsafe void RunNodeApi(RunNodeApiCallback runNodeApi)
124126

125127
using FunctorRef<node_embedding_node_api_run_callback> functorRef =
126128
CreateNodeApiRunFunctorRef(runNodeApi);
127-
JSRuntime.EmbeddingRuntimeRunNodeApi(Handle, functorRef.Callback, functorRef.Data)
129+
NodeEmbedding.JSRuntime.EmbeddingRuntimeRunNodeApi(
130+
Handle, functorRef.Callback, functorRef.Data)
128131
.ThrowIfFailed();
129132
}
130133

@@ -152,7 +155,7 @@ private static unsafe NodeEmbeddingStatus ReleaseRuntimeCallbackAdapter(nint dat
152155
}
153156
catch (Exception ex)
154157
{
155-
JSRuntime.EmbeddingSetLastErrorMessage(ex.Message.AsSpan());
158+
NodeEmbedding.JSRuntime.EmbeddingSetLastErrorMessage(ex.Message.AsSpan());
156159
return NodeEmbeddingStatus.GenericError;
157160
}
158161
}

src/NodeApi/Runtime/NodeEmbeddingRuntimeSettings.cs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,33 +23,31 @@ public class NodeEmbeddingRuntimeSettings
2323
public PostTaskCallback? OnPostTask { get; set; }
2424
public ConfigureRuntimeCallback? ConfigureRuntime { get; set; }
2525

26-
public static JSRuntime JSRuntime => NodeEmbedding.JSRuntime;
27-
2826
public unsafe ConfigureRuntimeCallback CreateConfigureRuntimeCallback()
2927
=> new((platform, config) =>
3028
{
3129
if (NodeApiVersion != null)
3230
{
33-
JSRuntime.EmbeddingRuntimeConfigSetNodeApiVersion(
31+
NodeEmbedding.JSRuntime.EmbeddingRuntimeConfigSetNodeApiVersion(
3432
config, NodeApiVersion.Value)
3533
.ThrowIfFailed();
3634
}
3735
if (RuntimeFlags != null)
3836
{
39-
JSRuntime.EmbeddingRuntimeConfigSetFlags(config, RuntimeFlags.Value)
37+
NodeEmbedding.JSRuntime.EmbeddingRuntimeConfigSetFlags(config, RuntimeFlags.Value)
4038
.ThrowIfFailed();
4139
}
4240
if (Args != null || RuntimeArgs != null)
4341
{
44-
JSRuntime.EmbeddingRuntimeConfigSetArgs(config, Args, RuntimeArgs)
42+
NodeEmbedding.JSRuntime.EmbeddingRuntimeConfigSetArgs(config, Args, RuntimeArgs)
4543
.ThrowIfFailed();
4644
}
4745

4846
if (OnPreload != null)
4947
{
5048
Functor<node_embedding_runtime_preload_callback> functor =
5149
CreateRuntimePreloadFunctor(OnPreload);
52-
JSRuntime.EmbeddingRuntimeConfigOnPreload(
50+
NodeEmbedding.JSRuntime.EmbeddingRuntimeConfigOnPreload(
5351
config, functor.Callback, functor.Data, functor.DataRelease)
5452
.ThrowIfFailed();
5553
}
@@ -64,15 +62,15 @@ JSValue onLoading(NodeEmbeddingRuntime runtime,
6462

6563
Functor<node_embedding_runtime_loading_callback> functor =
6664
CreateRuntimeLoadingFunctor(onLoading);
67-
JSRuntime.EmbeddingRuntimeConfigOnLoading(
65+
NodeEmbedding.JSRuntime.EmbeddingRuntimeConfigOnLoading(
6866
config, functor.Callback, functor.Data, functor.DataRelease)
6967
.ThrowIfFailed();
7068
}
7169
else if (OnLoading != null)
7270
{
7371
Functor<node_embedding_runtime_loading_callback> functor =
7472
CreateRuntimeLoadingFunctor(OnLoading);
75-
JSRuntime.EmbeddingRuntimeConfigOnLoading(
73+
NodeEmbedding.JSRuntime.EmbeddingRuntimeConfigOnLoading(
7674
config, functor.Callback, functor.Data, functor.DataRelease)
7775
.ThrowIfFailed();
7876
}
@@ -81,7 +79,7 @@ JSValue onLoading(NodeEmbeddingRuntime runtime,
8179
{
8280
Functor<node_embedding_runtime_loaded_callback> functor =
8381
CreateRuntimeLoadedFunctor(OnLoaded);
84-
JSRuntime.EmbeddingRuntimeConfigOnLoaded(
82+
NodeEmbedding.JSRuntime.EmbeddingRuntimeConfigOnLoaded(
8583
config, functor.Callback, functor.Data, functor.DataRelease)
8684
.ThrowIfFailed();
8785
}
@@ -92,7 +90,7 @@ JSValue onLoading(NodeEmbeddingRuntime runtime,
9290
{
9391
Functor<node_embedding_module_initialize_callback> functor =
9492
CreateModuleInitializeFunctor(module.OnInitialize);
95-
JSRuntime.EmbeddingRuntimeConfigAddModule(
93+
NodeEmbedding.JSRuntime.EmbeddingRuntimeConfigAddModule(
9694
config,
9795
module.Name.AsSpan(),
9896
functor.Callback,
@@ -107,7 +105,7 @@ JSValue onLoading(NodeEmbeddingRuntime runtime,
107105
{
108106
Functor<node_embedding_task_post_callback> functor =
109107
CreateTaskPostFunctor(OnPostTask);
110-
JSRuntime.EmbeddingRuntimeConfigSetTaskRunner(
108+
NodeEmbedding.JSRuntime.EmbeddingRuntimeConfigSetTaskRunner(
111109
config,
112110
new node_embedding_task_post_callback(s_taskPostCallback),
113111
(nint)GCHandle.Alloc(OnPostTask),

src/NodeApi/Runtime/NodeEmbeddingThreadRuntime.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,6 @@ internal NodeEmbeddingThreadRuntime(
8686
SynchronizationContext = syncContext;
8787
}
8888

89-
public static JSRuntime JSRuntime => NodeEmbedding.JSRuntime;
90-
9189
private static void InitializeModuleImportFunctions(
9290
JSRuntimeContext runtimeContext,
9391
string baseDir)

test/GCTests.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,13 @@ public class GCTests
1515
[Fact]
1616
public void GCHandles()
1717
{
18-
using NodeEmbeddingThreadRuntime nodejs = NodejsEmbeddingTests.CreateNodejsEnvironment();
18+
using NodeEmbeddingThreadRuntime nodejs =
19+
NodejsEmbeddingTests.CreateNodeEmbeddingThreadRuntime();
1920

2021
nodejs.Run(() =>
2122
{
23+
// 3 GC handles are created in the NodeEmbeddingThreadRuntime constructor
24+
// to define the 'require', 'resolve', and ' import' functions.
2225
Assert.Equal(3, JSRuntimeContext.Current.GCHandleCount);
2326

2427
JSClassBuilder<DotnetClass> classBuilder =
@@ -48,7 +51,7 @@ public void GCHandles()
4851
// Two more handles should have been allocated by the JS create-instance function call.
4952
// - One for the 'external' type value passed to the constructor.
5053
// - One for the JS object wrapper.
51-
Assert.Equal(3 + 7, JSRuntimeContext.Current.GCHandleCount);
54+
Assert.Equal(3 + 5 + 2, JSRuntimeContext.Current.GCHandleCount);
5255
});
5356

5457
nodejs.GC();
@@ -63,7 +66,8 @@ public void GCHandles()
6366
[Fact]
6467
public void GCObjects()
6568
{
66-
using NodeEmbeddingThreadRuntime nodejs = NodejsEmbeddingTests.CreateNodejsEnvironment();
69+
using NodeEmbeddingThreadRuntime nodejs =
70+
NodejsEmbeddingTests.CreateNodeEmbeddingThreadRuntime();
6771

6872
nodejs.Run(() =>
6973
{

0 commit comments

Comments
 (0)