Problem
The TS-side Runtime class has bookkeeping logic to track creation and disposal of native resources (NativeConnection, Worker, EphemeralServer, etc). That logic was introduced in the early days of the TS SDK to ensure that the native-side runtime thread would get properly terminated once all native resources had been released.
That bookkeeping is no longer required since the bridge rewrite in #1698, but removal of the mechanism had been left as a follow-up item, and never got completed.
The tracking logic is subject to race conditions that may, given specific call sequences, result in lang-side attempting to explicitly close resources that are still referenced and/or to use the native runtime handle after it has been cleared on the lang-side.
For various reasons, it is believed that such occurrences are very unlikely to happen in actual user code; we have however seen this problematic sequence of calls in some of our internal SDK tests, resulting in mysterious flakes, such as:
IllegalStateError: Cannot close connection while Workers hold a reference to it, thrown from NativeConnection.close during TestWorkflowEnvironment.teardown;NativePromiseDroppedError: Native Promise was dropped without being settled;- cascading
Error: Channel has been shut down failures from gRPC after the runtime is torn down underneath in-flight calls.
Background
Since the bridge refactor in #1698, the Core bridge owns the lifecycle of native resources (Runtime, Client, Worker, EphemeralServer) through the "Opaque Handle" model. That model guarantees proper cleanup of those resources, including when the JS side fails to call close/shutdown and the handle is simply dropped and garbage-collected.
Proposed change
Patching the language-side bookkeeping is possible, but is the wrong direction: the bookkeeping itself is obsolete and undesirable.
The proposed change is therefore to complete removal of the language-side resource tracking from Runtime:
- Drop
pendingCreations, backRefs, isIdle(), shutdownIfIdle(), and the createNative* wrappers;
- Simplify
closeNativeClient, deregisterWorker, shutdownEphemeralServer, and the register* / create* methods to no longer participate in that bookkeeping;
- Re-evaluate the explicit
native.runtimeShutdown(this.native) call in Runtime.shutdown() (the remaining FIXME); it most likely is no longer needed and should be removed alongside the rest.
The Runtime singleton will no longer auto-shut-down when the last Client / Worker / EphemeralServer is released. This is consistent with how other Core-based SDKs behave, and the post-#1698 ownership model ensures native resources are still cleaned up correctly.
Some more thinking will be required to clarify the impact of this cleanup on the way that the Runtime's instance global variable keeps a reference to the native- runtime, e.g. do we even need lang to shutdown the native runtime at any point?
Problem
The TS-side
Runtimeclass has bookkeeping logic to track creation and disposal of native resources (NativeConnection,Worker,EphemeralServer, etc). That logic was introduced in the early days of the TS SDK to ensure that the native-side runtime thread would get properly terminated once all native resources had been released.That bookkeeping is no longer required since the bridge rewrite in #1698, but removal of the mechanism had been left as a follow-up item, and never got completed.
The tracking logic is subject to race conditions that may, given specific call sequences, result in lang-side attempting to explicitly close resources that are still referenced and/or to use the native runtime handle after it has been cleared on the lang-side.
For various reasons, it is believed that such occurrences are very unlikely to happen in actual user code; we have however seen this problematic sequence of calls in some of our internal SDK tests, resulting in mysterious flakes, such as:
IllegalStateError: Cannot close connection while Workers hold a reference to it, thrown fromNativeConnection.closeduringTestWorkflowEnvironment.teardown;NativePromiseDroppedError: Native Promise was dropped without being settled;Error: Channel has been shut downfailures from gRPC after the runtime is torn down underneath in-flight calls.Background
Since the bridge refactor in #1698, the Core bridge owns the lifecycle of native resources (
Runtime,Client,Worker,EphemeralServer) through the "Opaque Handle" model. That model guarantees proper cleanup of those resources, including when the JS side fails to callclose/shutdownand the handle is simply dropped and garbage-collected.Proposed change
Patching the language-side bookkeeping is possible, but is the wrong direction: the bookkeeping itself is obsolete and undesirable.
The proposed change is therefore to complete removal of the language-side resource tracking from
Runtime:pendingCreations,backRefs,isIdle(),shutdownIfIdle(), and thecreateNative*wrappers;closeNativeClient,deregisterWorker,shutdownEphemeralServer, and theregister*/create*methods to no longer participate in that bookkeeping;native.runtimeShutdown(this.native)call inRuntime.shutdown()(the remainingFIXME); it most likely is no longer needed and should be removed alongside the rest.The
Runtimesingleton will no longer auto-shut-down when the lastClient/Worker/EphemeralServeris released. This is consistent with how other Core-based SDKs behave, and the post-#1698 ownership model ensures native resources are still cleaned up correctly.Some more thinking will be required to clarify the impact of this cleanup on the way that the Runtime's
instanceglobal variable keeps a reference to the native- runtime, e.g. do we even need lang to shutdown the native runtime at any point?