Avoid double Client SafeHandle implementation

Author: jmaeagle99

src/Temporalio/Bridge/Client.cs

@@ -1,6 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.Runtime.InteropServices;
 using System.Threading.Tasks;
 using Google.Protobuf;
 
@@ -9,18 +8,16 @@ namespace Temporalio.Bridge
     /// <summary>
     /// Core-owned Temporal client.
     /// </summary>
-    internal class Client : SafeHandle
+    internal class Client : IDisposable
     {
+        private readonly SafeClientHandle handle;
+
         private unsafe Client(Runtime runtime, Interop.TemporalCoreClient* ptr)
-            : base((IntPtr)ptr, true)
         {
             Runtime = runtime;
-            Handle = new SafeClientHandle(ptr);
+            handle = new SafeClientHandle(ptr);
         }
 
-        /// <inheritdoc />
-        public override bool IsInvalid => Handle.IsInvalid;
-
         /// <summary>
         /// Gets the runtime associated with this client.
         /// </summary>
@@ -29,7 +26,7 @@ private unsafe Client(Runtime runtime, Interop.TemporalCoreClient* ptr)
         /// <summary>
         /// Gets the safe handle for the client.
         /// </summary>
-        internal SafeClientHandle Handle { get; private init; }
+        internal SafeClientHandle Handle => handle;
 
         /// <summary>
         /// Connect to Temporal.
@@ -211,10 +208,9 @@ public async Task<T> CallAsync<T>(
         }
 
         /// <inheritdoc />
-        protected override unsafe bool ReleaseHandle()
+        public void Dispose()
         {
-            Handle.Dispose();
-            return true;
+            handle.Dispose();
         }
     }
 }

src/Temporalio/Bridge/Worker.cs

@@ -16,7 +16,8 @@ internal class Worker : IDisposable
         /// <summary>
         /// Initializes a new instance of the <see cref="Worker"/> class.
         /// </summary>
-        /// <param name="client">Client for the worker.</param>
+        /// <param name="runtime">Runtime for the worker.</param>
+        /// <param name="clientHandle">Client handle for the worker.</param>
         /// <param name="namespace">Namespace for the worker.</param>
         /// <param name="options">Options for the worker.</param>
         /// <param name="loggerFactory">Logger factory, used instead of the one in options by
@@ -26,12 +27,13 @@ internal class Worker : IDisposable
         /// If any of the options are invalid including improperly defined workflows/activities.
         /// </exception>
         public Worker(
-            Client client,
+            Runtime runtime,
+            SafeClientHandle clientHandle,
             string @namespace,
             Temporalio.Worker.TemporalWorkerOptions options,
             ILoggerFactory loggerFactory,
             IReadOnlyCollection<Temporalio.Client.ITemporalClientPlugin>? clientPlugins = null)
-            : this(client.Runtime, CreateWorkerHandle(client, @namespace, options, loggerFactory, clientPlugins))
+            : this(runtime, CreateWorkerHandle(runtime, clientHandle, @namespace, options, loggerFactory, clientPlugins))
         {
         }
 
@@ -98,16 +100,16 @@ public async Task ValidateAsync()
         /// <summary>
         /// Replace the client.
         /// </summary>
-        /// <param name="client">New client.</param>
-        public void ReplaceClient(Client client)
+        /// <param name="clientHandle">New client handle.</param>
+        public void ReplaceClient(SafeClientHandle clientHandle)
         {
             using (var scope = new Scope())
             {
                 unsafe
                 {
                     Interop.Methods.temporal_core_worker_replace_client(
                         scope.Pointer(Handle),
-                        scope.Pointer(client.Handle));
+                        scope.Pointer(clientHandle));
                 }
             }
         }
@@ -434,7 +436,8 @@ public void Dispose()
         }
 
         private static unsafe SafeHandleReference<SafeWorkerHandle> CreateWorkerHandle(
-            Client client,
+            Runtime runtime,
+            SafeClientHandle clientHandle,
             string @namespace,
             Temporalio.Worker.TemporalWorkerOptions options,
             ILoggerFactory loggerFactory,
@@ -443,13 +446,13 @@ private static unsafe SafeHandleReference<SafeWorkerHandle> CreateWorkerHandle(
             using Scope scope = new();
 
             var workerOrFail = Interop.Methods.temporal_core_worker_new(
-                scope.Pointer(client.Handle),
+                scope.Pointer(clientHandle),
                 scope.Pointer(options.ToInteropOptions(scope, @namespace, loggerFactory, clientPlugins)));
 
             if (workerOrFail.fail != null)
             {
                 string failStr;
-                using (var byteArray = new ByteArray(client.Runtime, workerOrFail.fail))
+                using (var byteArray = new ByteArray(runtime, workerOrFail.fail))
                 {
                     failStr = byteArray.ToUTF8();
                 }

src/Temporalio/Client/IBridgeClientProviderInternal.cs

@@ -0,0 +1,18 @@
+namespace Temporalio.Client
+{
+    /// <summary>
+    /// Internal interface that provides access to bridge client and runtime.
+    /// </summary>
+    internal interface IBridgeClientProviderInternal : IBridgeClientProvider
+    {
+        /// <summary>
+        /// Gets the safe client handle for internal bridge use.
+        /// </summary>
+        Bridge.SafeClientHandle? ClientHandle { get; }
+
+        /// <summary>
+        /// Gets the runtime associated with the client.
+        /// </summary>
+        Bridge.Runtime? Runtime { get; }
+    }
+}

src/Temporalio/Client/TemporalConnection.cs

@@ -18,7 +18,7 @@ namespace Temporalio.Client
     /// <remarks>
     /// Connections are thread-safe and are encouraged to be reused.
     /// </remarks>
-    public sealed class TemporalConnection : ITemporalConnection
+    public sealed class TemporalConnection : ITemporalConnection, IBridgeClientProviderInternal
     {
         // Not set if not lazy
         private readonly SemaphoreSlim? semaphoreForLazyClient;
@@ -177,7 +177,13 @@ public string? ApiKey
         public bool IsConnected => client != null;
 
         /// <inheritdoc />
-        public SafeHandle? BridgeClient => client;
+        public SafeHandle? BridgeClient => client?.Handle;
+
+        /// <inheritdoc />
+        Bridge.SafeClientHandle? IBridgeClientProviderInternal.ClientHandle => client?.Handle;
+
+        /// <inheritdoc />
+        Bridge.Runtime? IBridgeClientProviderInternal.Runtime => client?.Runtime;
 
         /// <summary>
         /// Connect to Temporal.

src/Temporalio/Worker/TemporalWorker.cs

@@ -61,10 +61,15 @@ public TemporalWorker(IWorkerClient client, TemporalWorkerOptions options)
                 // Ensure later accesses use the modified version of options.
                 options = Options;
 
-                var bridgeClient = client.BridgeClientProvider.BridgeClient ??
-                                   throw new InvalidOperationException("Cannot use unconnected lazy client for worker");
+                // Get runtime and client handle from bridge client provider
+                var bridgeProvider = client.BridgeClientProvider as Client.IBridgeClientProviderInternal
+                    ?? throw new InvalidOperationException("Client does not support internal bridge access");
+                var runtime = bridgeProvider.Runtime ?? Runtime.TemporalRuntime.Default.Runtime;
+                var clientHandle = bridgeProvider.ClientHandle
+                    ?? throw new InvalidOperationException("Cannot use unconnected lazy client for worker");
                 BridgeWorker = new(
-                    (Bridge.Client)bridgeClient,
+                    runtime,
+                    clientHandle,
                     client.Options.Namespace,
                     options,
                     loggerFactory,
@@ -181,11 +186,13 @@ public IWorkerClient Client
 
             set
             {
-                var bridgeClient = value.BridgeClientProvider.BridgeClient ??
-                    throw new InvalidOperationException("Cannot use unconnected lazy client for worker");
+                var bridgeProvider = value.BridgeClientProvider as Client.IBridgeClientProviderInternal
+                    ?? throw new InvalidOperationException("Client does not support internal bridge access");
+                var clientHandle = bridgeProvider.ClientHandle
+                    ?? throw new InvalidOperationException("Cannot use unconnected lazy client for worker");
                 lock (clientLock)
                 {
-                    BridgeWorker.ReplaceClient((Bridge.Client)bridgeClient);
+                    BridgeWorker.ReplaceClient(clientHandle);
                     client = value;
                 }
             }