Skip to content

Commit 2eb6c92

Browse files
committed
Address .NET SDK review feedback
1 parent b59944a commit 2eb6c92

12 files changed

Lines changed: 127 additions & 35 deletions

File tree

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ jobs:
114114
components: rustfmt, clippy
115115
rustflags: ""
116116

117-
- uses: actions/setup-dotnet@v4
117+
- uses: actions/setup-dotnet@c2fa09f4bde5ebb9d1777cf28262a3eb3db3ced7 # v5.2.0
118118
with:
119119
dotnet-version: '8.0.x'
120120

.github/workflows/publish.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ jobs:
128128
cache-key: release
129129
rustflags: ""
130130

131-
- uses: actions/setup-dotnet@v4
131+
- uses: actions/setup-dotnet@c2fa09f4bde5ebb9d1777cf28262a3eb3db3ced7 # v5.2.0
132132
with:
133133
dotnet-version: '8.0.x'
134134

src/sdk/dotnet/core/Api/HyperlightSandbox.Api.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
</ItemGroup>
2626

2727
<ItemGroup>
28+
<InternalsVisibleTo Include="HyperlightSandbox.Extensions.AI" />
2829
<InternalsVisibleTo Include="HyperlightSandbox.Tests" />
2930
</ItemGroup>
3031

src/sdk/dotnet/core/Api/Sandbox.cs

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -523,9 +523,9 @@ public Task RestoreAsync(SandboxSnapshot snapshot, CancellationToken cancellatio
523523
/// Releases the sandbox and all associated native resources.
524524
/// </summary>
525525
/// <remarks>
526-
/// After disposal, all pinned tool callback delegates are freed,
527-
/// allowing the GC to collect them. The native sandbox handle is
528-
/// also released.
526+
/// The native sandbox handle is released before pinned tool callback
527+
/// delegates are freed. This keeps callbacks valid for the full native
528+
/// drop path if the backend ever invokes them during cleanup.
529529
/// </remarks>
530530
public void Dispose()
531531
{
@@ -538,28 +538,34 @@ public void Dispose()
538538

539539
_disposed = true;
540540

541+
ReleaseNativeHandle();
541542
FreePinnedDelegates();
542543

543-
// Release the native sandbox handle.
544-
if (!_handle.IsInvalid)
545-
{
546-
_handle.Dispose();
547-
}
548-
549544
GC.SuppressFinalize(this);
550545
} // lock
551546
}
552547

553548
/// <summary>
554-
/// Destructor — ensures pinned GCHandles are freed even if the user
555-
/// forgets to call <see cref="Dispose"/>. The <see cref="SandboxSafeHandle"/>
556-
/// has its own finalizer for the native handle.
549+
/// Destructor — ensures the native handle is released before pinned
550+
/// GCHandles are freed even if the user forgets to call <see cref="Dispose"/>.
557551
/// </summary>
558552
~Sandbox()
559553
{
554+
ReleaseNativeHandle();
560555
FreePinnedDelegates();
561556
}
562557

558+
/// <summary>
559+
/// Releases the native sandbox handle. Safe to call multiple times.
560+
/// </summary>
561+
private void ReleaseNativeHandle()
562+
{
563+
if (!_handle.IsInvalid)
564+
{
565+
_handle.Dispose();
566+
}
567+
}
568+
563569
/// <summary>
564570
/// Frees all pinned tool callback delegates.
565571
/// Safe to call multiple times (idempotent).

src/sdk/dotnet/core/Api/SandboxBuilder.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ public sealed class SandboxBuilder
2929
private bool _tempOutput;
3030
private SandboxBackend _backend = SandboxBackend.Wasm;
3131

32+
/// <summary>
33+
/// Gets the backend configured for sandboxes built by this builder.
34+
/// </summary>
35+
internal SandboxBackend Backend => _backend;
36+
3237
/// <summary>
3338
/// Sets the backend to use. Default is <see cref="SandboxBackend.Wasm"/>.
3439
/// </summary>

src/sdk/dotnet/core/Extensions.AI/CodeExecutionTool.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,11 @@ namespace HyperlightSandbox.Extensions.AI;
3737
/// </remarks>
3838
public sealed class CodeExecutionTool : IDisposable
3939
{
40+
private const string WasmInitializationCode = "None";
41+
private const string JavaScriptInitializationCode = "void 0;";
42+
4043
private readonly Api.Sandbox _sandbox;
44+
private readonly string _initializationCode;
4145
private Api.SandboxSnapshot? _snapshot;
4246
private bool _initialized;
4347
private bool _disposed;
@@ -53,6 +57,7 @@ public sealed class CodeExecutionTool : IDisposable
5357
public CodeExecutionTool(Api.SandboxBuilder builder)
5458
{
5559
ArgumentNullException.ThrowIfNull(builder);
60+
_initializationCode = InitializationCodeFor(builder.Backend);
5661
_sandbox = builder.Build();
5762
}
5863

@@ -140,8 +145,7 @@ public Api.ExecutionResult Execute(string code)
140145
{
141146
// Initialize the sandbox runtime with a no-op, then snapshot
142147
// the CLEAN state before any user code pollutes it.
143-
// Use empty string that's valid in both Python ("") and JS ("").
144-
_sandbox.Run("None");
148+
_sandbox.Run(_initializationCode);
145149
_snapshot = _sandbox.Snapshot();
146150
_initialized = true;
147151
}
@@ -222,4 +226,11 @@ public void Dispose()
222226
// Only free what WE own. The sandbox cleans up via its own finalizer.
223227
_snapshot?.Dispose();
224228
}
229+
230+
internal static string InitializationCodeFor(Api.SandboxBackend backend) => backend switch
231+
{
232+
Api.SandboxBackend.Wasm => WasmInitializationCode,
233+
Api.SandboxBackend.JavaScript => JavaScriptInitializationCode,
234+
_ => throw new ArgumentOutOfRangeException(nameof(backend), backend, "Unknown sandbox backend."),
235+
};
225236
}

src/sdk/dotnet/core/Extensions.AI/HyperlightSandbox.Extensions.AI.csproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,8 @@
2828
<PackageReference Include="Microsoft.Extensions.AI" Version="9.5.0" />
2929
</ItemGroup>
3030

31+
<ItemGroup>
32+
<InternalsVisibleTo Include="HyperlightSandbox.Tests" />
33+
</ItemGroup>
34+
3135
</Project>

src/sdk/dotnet/core/PInvoke/SafeNativeMethods.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ private static IntPtr DllImportResolver(
6767
string rid = OperatingSystem.IsWindows() ? "win-x64" : "linux-x64";
6868
string runtimePath = Path.Join(
6969
assemblyDirectory, "runtimes", rid, "native", platformLibraryName);
70+
List<string> searchedPaths = [runtimePath];
7071

7172
if (File.Exists(runtimePath))
7273
{
@@ -75,6 +76,7 @@ private static IntPtr DllImportResolver(
7576

7677
// Check assembly directory directly (local development)
7778
string localPath = Path.Join(assemblyDirectory, platformLibraryName);
79+
searchedPaths.Add(localPath);
7880
if (File.Exists(localPath))
7981
{
8082
return NativeLibrary.Load(localPath);
@@ -87,12 +89,14 @@ private static IntPtr DllImportResolver(
8789
while (dir != null)
8890
{
8991
string cargoTarget = Path.Join(dir, "target", "debug", platformLibraryName);
92+
searchedPaths.Add(cargoTarget);
9093
if (File.Exists(cargoTarget))
9194
{
9295
return NativeLibrary.Load(cargoTarget);
9396
}
9497

9598
string cargoTargetRelease = Path.Join(dir, "target", "release", platformLibraryName);
99+
searchedPaths.Add(cargoTargetRelease);
96100
if (File.Exists(cargoTargetRelease))
97101
{
98102
return NativeLibrary.Load(cargoTargetRelease);
@@ -102,8 +106,9 @@ private static IntPtr DllImportResolver(
102106
}
103107
#endif
104108

105-
// Fallback to default resolution
106-
return IntPtr.Zero;
109+
throw new DllNotFoundException(
110+
$"Unable to load native library '{libraryName}'. Searched paths: " +
111+
string.Join(Path.PathSeparator, searchedPaths));
107112
}
108113

109114
// -----------------------------------------------------------------------

src/sdk/dotnet/core/Tests/HyperlightSandbox.Tests/CodeExecutionToolTests.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ public void Create_WithBuilder_Succeeds()
2525
Assert.NotNull(tool);
2626
}
2727

28+
[Fact]
29+
public void Create_WithJavaScriptBackend_Succeeds()
30+
{
31+
using var tool = new CodeExecutionTool(
32+
new SandboxBuilder().WithBackend(SandboxBackend.JavaScript));
33+
34+
Assert.NotNull(tool);
35+
}
36+
2837
[Fact]
2938
public void Create_NullBuilder_ThrowsArgumentNullException()
3039
{
@@ -51,6 +60,16 @@ public void Execute_AfterDispose_ThrowsObjectDisposedException()
5160
tool.Execute("print('hello')"));
5261
}
5362

63+
[Theory]
64+
[InlineData(SandboxBackend.Wasm, "None")]
65+
[InlineData(SandboxBackend.JavaScript, "void 0;")]
66+
public void InitializationCodeFor_UsesBackendNoOp(
67+
SandboxBackend backend,
68+
string expectedCode)
69+
{
70+
Assert.Equal(expectedCode, CodeExecutionTool.InitializationCodeFor(backend));
71+
}
72+
5473
// -----------------------------------------------------------------------
5574
// Tool registration
5675
// -----------------------------------------------------------------------

src/sdk/dotnet/core/Tests/HyperlightSandbox.Tests/JavaScriptBackendTests.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,19 @@ public void WithBackend_Default_IsWasm()
4949
{
5050
// Default backend requires module path (Wasm)
5151
var builder = new SandboxBuilder();
52+
Assert.Equal(SandboxBackend.Wasm, builder.Backend);
5253
Assert.Throws<InvalidOperationException>(() => builder.Build());
5354
}
5455

56+
[Fact]
57+
public void WithBackend_UpdatesBackendProperty()
58+
{
59+
var builder = new SandboxBuilder()
60+
.WithBackend(SandboxBackend.JavaScript);
61+
62+
Assert.Equal(SandboxBackend.JavaScript, builder.Backend);
63+
}
64+
5565
// -----------------------------------------------------------------------
5666
// Lifecycle
5767
// -----------------------------------------------------------------------

0 commit comments

Comments
 (0)