Skip to content

Stop freeing native resources in finalizers #582

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.

Sign up for GitHub

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

Draft
cretz wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Stop freeing native resources in finalizers #582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
939866b
Stop freeing native resources in finalizers
cretz Dec 19, 2025
9506d40
Work on lifetimes for handles
cretz Dec 20, 2025
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
2 changes: 1 addition & 1 deletion Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<Nullable>enable</Nullable>
<PackageLicenseExpression>MIT</PackageLicenseExpression>
<RepositoryUrl>https://github.com/temporalio/sdk-dotnet</RepositoryUrl>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<TreatWarningsAsErrors>false</TreatWarningsAsErrors>
<ProduceReferenceAssembly>True</ProduceReferenceAssembly>
</PropertyGroup>

Expand Down
29 changes: 19 additions & 10 deletions src/Temporalio/Bridge/ByteArrayRef.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Temporalio.Bridge
/// Representation of a byte array owned by .NET. Users should usually use a
/// <see cref="Scope" /> instead of creating this directly.
/// </summary>
internal class ByteArrayRef
internal sealed class ByteArrayRef : IDisposable
{
private readonly GCHandle bytesHandle;

Expand Down Expand Up @@ -42,18 +42,10 @@ public ByteArrayRef(byte[] bytes, int length)
}
}

/// <summary>
/// Finalizes an instance of the <see cref="ByteArrayRef"/> class.
/// </summary>
~ByteArrayRef()
{
bytesHandle.Free();
}

/// <summary>
/// Gets empty byte array.
/// </summary>
public static ByteArrayRef Empty { get; } = new(Array.Empty<byte>());
public static ByteArrayRef Empty { get; } = new(Array.Empty<byte>()) { DisableDispose = true };

/// <summary>
/// Gets current byte array for this ref.
Expand All @@ -70,6 +62,8 @@ public ByteArrayRef(byte[] bytes, int length)
/// </summary>
internal static UTF8Encoding StrictUTF8 { get; } = new(false, true);

private bool DisableDispose { get; set; }

/// <summary>
/// Convert a string to a UTF-8 byte array.
/// </summary>
Expand Down Expand Up @@ -210,5 +204,20 @@ public static ByteArrayRef FromNewlineDelimited(IEnumerable<string> values)
return new ByteArrayRef(stream.GetBuffer(), (int)stream.Length);
}
}

/// <inheritdoc/>
public void Dispose()
{
// Note, we intentionally do not free or call Dispose from a finalizer. When working
// with native objects, best practice is to not free in finalizer. On process shutdown,
// finalizers can be called even when the object is still referenced.

// Does not need to be thread safe
if (!DisableDispose)
{
DisableDispose = true;
bytesHandle.Free();
}
}
}
}
102 changes: 49 additions & 53 deletions src/Temporalio/Bridge/EnvConfig.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,74 +18,70 @@ internal static class EnvConfig
/// <param name="options">Options for loading the configuration.</param>
/// <returns>Dictionary of profile name to client configuration profile.</returns>
public static Dictionary<string, ClientEnvConfig.Profile> LoadClientConfig(
ClientEnvConfig.ConfigLoadOptions options)
{
using var scope = new Scope();

try
ClientEnvConfig.ConfigLoadOptions options) => Scope.WithScope(scope =>
{
var envVarsRef = options.OverrideEnvVars?.Count > 0
? scope.ByteArray(JsonSerializer.Serialize(options.OverrideEnvVars))
: ByteArrayRef.Empty.Ref;

unsafe
try
{
var coreOptions = new Interop.TemporalCoreClientEnvConfigLoadOptions
var envVarsRef = options.OverrideEnvVars?.Count > 0
? scope.ByteArray(JsonSerializer.Serialize(options.OverrideEnvVars))
: ByteArrayRef.Empty.Ref;

unsafe
{
path = scope.ByteArray(options.ConfigSource?.Path),
data = scope.ByteArray(options.ConfigSource?.Data),
config_file_strict = Convert.ToByte(options.ConfigFileStrict),
env_vars = envVarsRef,
};

var result = Interop.Methods.temporal_core_client_env_config_load(&coreOptions);
return ProcessAllProfilesResult(result);
var coreOptions = new Interop.TemporalCoreClientEnvConfigLoadOptions
{
path = scope.ByteArray(options.ConfigSource?.Path),
data = scope.ByteArray(options.ConfigSource?.Data),
config_file_strict = Convert.ToByte(options.ConfigFileStrict),
env_vars = envVarsRef,
};

var result = Interop.Methods.temporal_core_client_env_config_load(&coreOptions);
return ProcessAllProfilesResult(result);
}
}
}
catch (JsonException ex)
{
throw new InvalidOperationException($"Failed to deserialize client config: {ex.Message}", ex);
}
}
catch (JsonException ex)
{
throw new InvalidOperationException($"Failed to deserialize client config: {ex.Message}", ex);
}
});

/// <summary>
/// Loads a specific client configuration profile.
/// </summary>
/// <param name="options">Options for loading the configuration profile.</param>
/// <returns>Client configuration for the specified profile.</returns>
public static ClientEnvConfig.Profile LoadClientConfigProfile(
ClientEnvConfig.ProfileLoadOptions options)
{
using var scope = new Scope();

try
ClientEnvConfig.ProfileLoadOptions options) => Scope.WithScope(scope =>
{
var envVarsRef = options.OverrideEnvVars?.Count > 0
? scope.ByteArray(JsonSerializer.Serialize(options.OverrideEnvVars))
: ByteArrayRef.Empty.Ref;

unsafe
try
{
var coreOptions = new Interop.TemporalCoreClientEnvConfigProfileLoadOptions
var envVarsRef = options.OverrideEnvVars?.Count > 0
? scope.ByteArray(JsonSerializer.Serialize(options.OverrideEnvVars))
: ByteArrayRef.Empty.Ref;

unsafe
{
profile = scope.ByteArray(options.Profile),
path = scope.ByteArray(options.ConfigSource?.Path),
data = scope.ByteArray(options.ConfigSource?.Data),
disable_file = Convert.ToByte(options.DisableFile),
disable_env = Convert.ToByte(options.DisableEnv),
config_file_strict = Convert.ToByte(options.ConfigFileStrict),
env_vars = envVarsRef,
};

var result = Interop.Methods.temporal_core_client_env_config_profile_load(&coreOptions);
return ProcessSingleProfileResult(result);
var coreOptions = new Interop.TemporalCoreClientEnvConfigProfileLoadOptions
{
profile = scope.ByteArray(options.Profile),
path = scope.ByteArray(options.ConfigSource?.Path),
data = scope.ByteArray(options.ConfigSource?.Data),
disable_file = Convert.ToByte(options.DisableFile),
disable_env = Convert.ToByte(options.DisableEnv),
config_file_strict = Convert.ToByte(options.ConfigFileStrict),
env_vars = envVarsRef,
};

var result = Interop.Methods.temporal_core_client_env_config_profile_load(&coreOptions);
return ProcessSingleProfileResult(result);
}
}
}
catch (JsonException ex)
{
throw new InvalidOperationException($"Failed to deserialize client config profile: {ex.Message}", ex);
}
}
catch (JsonException ex)
{
throw new InvalidOperationException($"Failed to deserialize client config profile: {ex.Message}", ex);
}
});

/// <summary>
/// Creates a ClientConfigProfile from typed JSON data.
Expand Down
121 changes: 61 additions & 60 deletions src/Temporalio/Bridge/EphemeralServer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,85 +45,82 @@ private unsafe EphemeralServer(
/// <param name="runtime">Runtime to use.</param>
/// <param name="options">Options to use.</param>
/// <returns>Started server.</returns>
public static async Task<EphemeralServer> StartDevServerAsync(
public static Task<EphemeralServer> StartDevServerAsync(
Runtime runtime,
Testing.WorkflowEnvironmentStartLocalOptions options)
{
using (var scope = new Scope())
{
var completion = new TaskCompletionSource<EphemeralServer>(
TaskCreationOptions.RunContinuationsAsynchronously);
unsafe
Testing.WorkflowEnvironmentStartLocalOptions options) =>
Scope.WithScopeAsync<EphemeralServer>(
runtime,
scope =>
{
Interop.Methods.temporal_core_ephemeral_server_start_dev_server(
runtime.Ptr,
scope.Pointer(options.ToInteropOptions(scope)),
null,
CallbackForStart(runtime, scope, false, completion));
}
return await completion.Task.ConfigureAwait(false);
}
}
unsafe
{
Interop.Methods.temporal_core_ephemeral_server_start_dev_server(
runtime.Ptr,
scope.Pointer(options.ToInteropOptions(scope)),
null,
CallbackForStart(runtime, scope, false));
}
},
TaskCreationOptions.RunContinuationsAsynchronously);

/// <summary>
/// Start test server.
/// </summary>
/// <param name="runtime">Runtime to use.</param>
/// <param name="options">Options to use.</param>
/// <returns>Started server.</returns>
public static async Task<EphemeralServer> StartTestServerAsync(
public static Task<EphemeralServer> StartTestServerAsync(
Runtime runtime,
Testing.WorkflowEnvironmentStartTimeSkippingOptions options)
{
using (var scope = new Scope())
{
var completion = new TaskCompletionSource<EphemeralServer>(
TaskCreationOptions.RunContinuationsAsynchronously);
unsafe
Testing.WorkflowEnvironmentStartTimeSkippingOptions options) =>
Scope.WithScopeAsync<EphemeralServer>(
runtime,
scope =>
{
Interop.Methods.temporal_core_ephemeral_server_start_test_server(
runtime.Ptr,
scope.Pointer(options.ToInteropOptions(scope)),
null,
CallbackForStart(runtime, scope, true, completion));
}
return await completion.Task.ConfigureAwait(false);
}
}
unsafe
{
Interop.Methods.temporal_core_ephemeral_server_start_test_server(
runtime.Ptr,
scope.Pointer(options.ToInteropOptions(scope)),
null,
CallbackForStart(runtime, scope, true));
}
},
TaskCreationOptions.RunContinuationsAsynchronously);

/// <summary>
/// Shutdown the server.
/// </summary>
/// <returns>Task.</returns>
public async Task ShutdownAsync()
public Task ShutdownAsync() => Scope.WithScopeAsync<bool>(this, scope =>
{
using (var scope = new Scope())
unsafe
{
var completion = new TaskCompletionSource<bool>(
TaskCreationOptions.RunContinuationsAsynchronously);
unsafe
try
{
Interop.Methods.temporal_core_ephemeral_server_shutdown(
ptr,
null,
scope.FunctionPointer<Interop.TemporalCoreEphemeralServerShutdownCallback>(
scope.CallbackPointer<Interop.TemporalCoreEphemeralServerShutdownCallback>(
(userData, fail) =>
{
if (fail != null)
{
completion.TrySetException(
scope.Completion.TrySetException(
new InvalidOperationException(
new ByteArray(runtime, fail).ToUTF8()));
}
else
{
completion.TrySetResult(true);
scope.Completion.TrySetResult(true);
}
}));
}
await completion.Task.ConfigureAwait(false);
finally
{
scope.OnCallbackExit();
}
}
}
});

/// <inheritdoc />
protected override unsafe bool ReleaseHandle()
Expand All @@ -134,28 +131,32 @@ protected override unsafe bool ReleaseHandle()

private static unsafe IntPtr CallbackForStart(
Runtime runtime,
Scope scope,
bool hasTestService,
TaskCompletionSource<EphemeralServer> completion)
{
return scope.FunctionPointer<Interop.TemporalCoreEphemeralServerStartCallback>(
Scope.WithCallback<EphemeralServer> scope,
bool hasTestService) =>
scope.CallbackPointer<Interop.TemporalCoreEphemeralServerStartCallback>(
(userData, success, successTarget, fail) =>
{
if (fail != null)
try
{
completion.TrySetException(
new InvalidOperationException(new ByteArray(runtime, fail).ToUTF8()));
if (fail != null)
{
scope.Completion.TrySetException(
new InvalidOperationException(new ByteArray(runtime, fail).ToUTF8()));
}
else
{
scope.Completion.TrySetResult(
new EphemeralServer(
runtime,
success,
new ByteArray(runtime, successTarget).ToUTF8(),
hasTestService));
}
}
else
finally
{
completion.TrySetResult(
new EphemeralServer(
runtime,
success,
new ByteArray(runtime, successTarget).ToUTF8(),
hasTestService));
scope.OnCallbackExit();
}
});
}
}
}
6 changes: 3 additions & 3 deletions src/Temporalio/Bridge/Metric.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace Temporalio.Bridge
/// </summary>
internal class Metric : SafeHandle
{
private readonly unsafe Interop.TemporalCoreMetric* ptr;
private unsafe Interop.TemporalCoreMetric* ptr;

/// <summary>
/// Initializes a new instance of the <see cref="Metric"/> class.
Expand All @@ -26,7 +26,7 @@ public Metric(
string? description)
: base(IntPtr.Zero, true)
{
using (var scope = new Scope())
Scope.WithScope(scope =>
{
unsafe
{
Expand All @@ -40,7 +40,7 @@ public Metric(
ptr = Interop.Methods.temporal_core_metric_new(meter.Ptr, scope.Pointer(options));
SetHandle((IntPtr)ptr);
}
}
});
}

/// <inheritdoc />
Expand Down
Loading
Loading