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

Update for Node.js embedding API changes #219

Merged
merged 2 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ jobs:
continue-on-error: true

- name: Upload test logs
uses: actions/upload-artifact@v4
# upload-artifact@v4 breaks the test reporter: https://github.com/dorny/test-reporter/issues/343
uses: actions/upload-artifact@v3
with:
name: test-logs-${{ matrix.os }}-${{matrix.dotnet-version}}-node${{matrix.node-version}}-${{matrix.configuration}}
path: |
Expand Down
3 changes: 2 additions & 1 deletion bench/Benchmarks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ protected void Setup()

// This setup avoids using NodejsEnvironment so benchmarks can run on the same thread.
// NodejsEnvironment creates a separate thread that would slow down the micro-benchmarks.
platform.Runtime.CreateEnvironment(platform, Console.WriteLine, null, out _env)
platform.Runtime.CreateEnvironment(
platform, Console.WriteLine, null, NodejsEnvironment.NodeApiVersion, out _env)
.ThrowIfFailed();

// The new scope instance saves itself as the thread-local JSValueScope.Current.
Expand Down
2 changes: 1 addition & 1 deletion src/NodeApi/Runtime/JSRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -521,14 +521,14 @@ public virtual napi_status GetBufferInfo(

public virtual napi_status CreatePlatform(
string[]? args,
string[]? execArgs,
Action<string>? errorHandler,
out napi_platform result) => throw NS();
public virtual napi_status DestroyPlatform(napi_platform platform) => throw NS();
public virtual napi_status CreateEnvironment(
napi_platform platform,
Action<string>? errorHandler,
string? mainScript,
int apiVersion,
out napi_env result) => throw NS();
public virtual napi_status DestroyEnvironment(napi_env env, out int exitCode) => throw NS();
public virtual napi_status RunEnvironment(napi_env env) => throw NS();
Expand Down
6 changes: 6 additions & 0 deletions src/NodeApi/Runtime/NodejsEnvironment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ namespace Microsoft.JavaScript.NodeApi.Runtime;
/// </remarks>
public sealed class NodejsEnvironment : IDisposable
{
/// <summary>
/// Corresponds to NAPI_VERSION from js_native_api.h.
/// </summary>
public const int NodeApiVersion = 8;

private readonly JSValueScope _scope;
private readonly Thread _thread;
private readonly TaskCompletionSource<bool> _completion = new();
Expand All @@ -44,6 +49,7 @@ internal NodejsEnvironment(NodejsPlatform platform, string? mainScript)
(napi_platform)platform,
(error) => Console.WriteLine(error),
mainScript,
NodeApiVersion,
out napi_env env).ThrowIfFailed();

// The new scope instance saves itself as the thread-local JSValueScope.Current.
Expand Down
8 changes: 3 additions & 5 deletions src/NodeApi/Runtime/NodejsPlatform.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,12 @@ public sealed class NodejsPlatform : IDisposable
/// Initializes the Node.js platform.
/// </summary>
/// <param name="libnodePath">Path to the `libnode` shared library, including extension.</param>
/// <param name="args">Optional application arguments.</param>
/// <param name="execArgs">Optional platform options.</param>
/// <param name="args">Optional platform arguments.</param>
/// <exception cref="InvalidOperationException">A Node.js platform instance has already been
/// loaded in the current process.</exception>
public NodejsPlatform(
string libnodePath,
string[]? args = null,
string[]? execArgs = null)
string[]? args = null)
{
if (string.IsNullOrEmpty(libnodePath)) throw new ArgumentNullException(nameof(libnodePath));

Expand All @@ -46,7 +44,7 @@ public NodejsPlatform(
nint libnodeHandle = NativeLibrary.Load(libnodePath);
Runtime = new NodejsRuntime(libnodeHandle);

Runtime.CreatePlatform(args, execArgs, (error) => Console.WriteLine(error), out _platform)
Runtime.CreatePlatform(args, (error) => Console.WriteLine(error), out _platform)
.ThrowIfFailed();
Current = this;
}
Expand Down
15 changes: 5 additions & 10 deletions src/NodeApi/Runtime/NodejsRuntime.Embedding.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,11 @@ public unsafe partial class NodejsRuntime
{
#pragma warning disable IDE1006 // Naming: missing prefix '_'

private delegate* unmanaged[Cdecl]<
int, nint, int, nint, napi_error_message_handler, nint, napi_status>
private delegate* unmanaged[Cdecl]<int, nint, napi_error_message_handler, nint, napi_status>
napi_create_platform;

public override napi_status CreatePlatform(
string[]? args,
string[]? execArgs,
Action<string>? errorHandler,
out napi_platform result)
{
Expand All @@ -29,7 +27,6 @@ public override napi_status CreatePlatform(
});

nint args_ptr = StringsToHGlobalUtf8(args, out int args_count);
nint exec_args_ptr = StringsToHGlobalUtf8(execArgs, out int exec_args_count);

try
{
Expand All @@ -39,23 +36,20 @@ public override napi_status CreatePlatform(
if (napi_create_platform == null)
{
napi_create_platform = (delegate* unmanaged[Cdecl]<
int, nint, int, nint, napi_error_message_handler, nint, napi_status>)
int, nint, napi_error_message_handler, nint, napi_status>)
Import(nameof(napi_create_platform));
}

return napi_create_platform(
args_count,
args_ptr,
exec_args_count,
exec_args_ptr,
native_error_handler,
(nint)result_ptr);
}
}
finally
{
FreeStringsHGlobal(args_ptr, args_count);
FreeStringsHGlobal(exec_args_ptr, exec_args_count);
}
}

Expand All @@ -68,13 +62,14 @@ public override napi_status DestroyPlatform(napi_platform platform)
}

private delegate* unmanaged[Cdecl]<
napi_platform, napi_error_message_handler, nint, nint, napi_status>
napi_platform, napi_error_message_handler, nint, int, nint, napi_status>
napi_create_environment;

public override napi_status CreateEnvironment(
napi_platform platform,
Action<string>? errorHandler,
string? mainScript,
int apiVersion,
out napi_env result)
{
napi_error_message_handler native_error_handler = errorHandler == null ? default :
Expand All @@ -91,7 +86,7 @@ public override napi_status CreateEnvironment(
fixed (napi_env* result_ptr = &result)
{
return Import(ref napi_create_environment)(
platform, native_error_handler, main_script_ptr, (nint)result_ptr);
platform, native_error_handler, main_script_ptr, apiVersion, (nint)result_ptr);
}
}
finally
Expand Down
9 changes: 5 additions & 4 deletions src/NodeApi/Runtime/TracingJSRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2648,15 +2648,14 @@ public override napi_status GetNodeVersion(napi_env env, out napi_node_version r
#region Embedding

public override napi_status CreatePlatform(
string[]? args, string[]? execArgs, Action<string>? errorHandler, out napi_platform result)
string[]? args, Action<string>? errorHandler, out napi_platform result)
{
napi_platform resultValue = default;
napi_status status = TraceCall(
[
$"[{string.Join(", ", args ?? [])}]",
$"[{string.Join(", ", execArgs ?? [])}]",
],
() => (_runtime.CreatePlatform(args, execArgs, errorHandler, out resultValue),
() => (_runtime.CreatePlatform(args, errorHandler, out resultValue),
Format(resultValue)));
result = resultValue;
return status;
Expand All @@ -2673,12 +2672,14 @@ public override napi_status CreateEnvironment(
napi_platform platform,
Action<string>? errorHandler,
string? mainScript,
int apiVersion,
out napi_env result)
{
napi_env resultValue = default;
napi_status status = TraceCall(
[Format(platform), Format(mainScript)],
() => (_runtime.CreateEnvironment(platform, errorHandler, mainScript, out resultValue),
() => (_runtime.CreateEnvironment(
platform, errorHandler, mainScript, apiVersion, out resultValue),
Format(resultValue)));
result = resultValue;
return status;
Expand Down
8 changes: 8 additions & 0 deletions test/NodejsEmbeddingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ public void NodejsStart()
Assert.Equal(0, nodejs.ExitCode);
}

[SkippableFact]
public void NodejsRestart()
{
// Create and destory a Node.js environment twice, using the same platform instance.
NodejsStart();
NodejsStart();
}

[SkippableFact]
public void NodejsCallFunction()
{
Expand Down
Loading