Skip to content

Conversation

@VegetarianOrc
Copy link
Contributor

@VegetarianOrc VegetarianOrc commented Nov 10, 2025

Raise an assertion error when a Runtime is used by client/worker creation/usage.

What was changed

  • Add fork checks to bridge runtime and raise a RuntimeError if the runtime was created by a different process.
  • Add new _RuntimeRef private class to encapsulate behavior around managing calls to Runtime.default
  • Replace global var of _default_runtime with an instance of the new _RuntimeRef and use it in calls to Runtime.default, Runtime.prevent_default and Runtime.set_default.

Why?

Startup time in large codebases can be challenge to manage. One strategy to manage startup time is to load modules once and then fork the process into many instances so the child process don't need to repeat the startup cost. These changes help users employing that strategy to manage Temporal usage across forks in a safe manner.

Checklist

  1. Closes [Feature Request] Better prevention of accidental Runtime use over forks #1201

  2. How was this tested:

  • Unit tests were added that create a fork and expect an assertion error for the following:
    • creating a client with Client.connect after Runtime.default was called pre-fork
    • using a client created pre-fork
    • creating a worker after Runtime.default was called pre-fork
    • using a worker created pre-fork
  • Unit tests were added to validate behavior of new _RuntimeRef class that the lifecycle of default runtime.
  1. Any docs updates needed?

@VegetarianOrc VegetarianOrc marked this pull request as ready for review November 12, 2025 22:47
@VegetarianOrc VegetarianOrc requested a review from a team as a code owner November 12, 2025 22:47
Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, non-blocking concern about if we need the entire new RuntimeRef class to add a global boolean

) -> None:
self._default_runtime: Runtime | None = None
self._prevent_default = False
self._default_created = False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this field needed if _default_runtime's None-ness can be used as basically the same thing? And therefore if you have default runtime and prevent default, do they deserve an entirely new class/encapsulation as opposed to just adding a single _prevent_default global alongside the existing global?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My original thinking for _default_created was to differentiate between the default being lazily created and one being set via set_default. I can see that not being worth tracking and just raising in prevent_default if an existing _default_runtime exists. I think I'll make that change because it similarly makes sense to avoid calling prevent_default after set_default. Even though that would be harmless, it is a weird thing to do.

The primary purpose of the encapsulation was just to write tests in a way that plays nicely with existing fixtures. Definitely happy to consider alternatives here.

Copy link
Member

@cretz cretz Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meh, if we have to extract out to a new class for test only, ok. It makes for a bit uglier stack trace for users, but not a big deal.

_default_runtime = runtime
global _runtime_ref
_runtime_ref.set_default(runtime, error_if_already_set=error_if_already_set)
return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] Better prevention of accidental Runtime use over forks

4 participants