Skip to content

Make simulations run on different thread #3657

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

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Make simulations run on different thread #3657

wants to merge 4 commits into from
+62 −69

Conversation

@Shourya742
Copy link
Contributor

Part of #1845 . This PR runs each simulation inside its own thread, allowing panics to be safely contained and recovered without affecting the main thread or global state. This is a first step toward isolating panics during panic simulation.

cc: @pedrocarlo

@Shourya742 Shourya742 requested a review from penberg as a code owner October 10, 2025 09:44
@Shourya742 Shourya742 changed the title Make simulator run on different thread Make simulations run on different thread Oct 10, 2025
@Shourya742
Copy link
Contributor Author

@penberg to run the simulator in a separate thread, I had to add unsafe Send + Sync impls for SimulatorEnv. I initially tried to avoid that, but since it includes the Connection object which isn’t thread-safe it couldn’t be passed around threads cleanly. The other option would’ve been to refactor the lib to replace single-threaded primitives with thread-safe ones, but that’s a bigger change, and we do rely on a few invariants that assume single-threaded behavior (some I might not even be fully aware of). So, adding the unsafe impls felt like the most reasonable workaround for now. Curious to hear your thoughts on this.

@nyrkio
Copy link

nyrkio bot commented Oct 13, 2025 •
edited
Loading

Nyrkiö Report for Commit: dafb003

No performance changes detected.

Remember that Nyrkiö results become more precise when more commits are merged. So please check back in a few days.

Nyrkiö

Shourya742 and others added 4 commits October 13, 2025 20:54
@Shourya742
simulator: add setup_panic_hook for unified panic logging
e073af2
@Shourya742
simulator: run simulations in isolated threads via SandboxedResult::from
d520716 
Replaces direct `catch_unwind` calls with a threaded sandbox model:
each simulation now runs in a dedicated thread, wrapped with
`AssertUnwindSafe` to satisfy unwind safety guarantees.
@Shourya742
simulator/env: mark SimulatorEnv as Send + Sync
811687a
@pedrocarlo @Shourya742
make Assertion Func Send + Sync
dafb003
@Shourya742 Shourya742 force-pushed the make_simulator_run_on_different_thread branch from 1c465fb to dafb003 Compare October 13, 2025 15:25
@Shourya742
Copy link
Contributor Author

cc: @penberg

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

Reviewers

@penberg penberg Awaiting requested review from penberg penberg is a code owner

Assignees

No one assigned

Labels

simulator

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Shourya742 @pedrocarlo