Skip to content

Do not throw from CUDA destructors and avoid implicit default streams in eval/compile - #4514

Merged
zcbenz merged 4 commits into
ml-explore:mainfrom
aleroot:threads_and_strems_4506
Sep 28, 2026
Merged

zcbenz merged 4 commits into
ml-explore:mainfrom
aleroot:threads_and_strems_4506

Conversation

@aleroot

@aleroot aleroot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

As described in #4506 a termination crash occurs when a detached worker thread outlives the CUDA runtime and attempts to clean up CUDA resources, triggering a std::terminate.

To resolve this I have made some changes so that the worker class is restructured therefore the detached thread only captures a new, inert State struct containing standard threading primitives. Additionally the CUDA resources remain in the Worker, which is now managed by the CommandEncoder as a unique_ptr to ensure deterministic destruction before the runtime unloads. The destructor ~CudaHandle has been modified to safely ignore errors during shutdown, and eval_impl now selects streams lazily to prevent accidental worker instantiation.

Fixes #4506

  • ☑️ I understand it is strictly prohibited to use AI to write PR description
  • AI usage disclosure:

@zcbenz zcbenz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think there is no need to change Worker, the crash in #4506 was caused by CommandEncoder getting destroyed after CUDA runtime shutdown and this change is not going to prevent that.

@aleroot

aleroot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

I think there is no need to change Worker, the crash in #4506 was caused by CommandEncoder getting destroyed after CUDA runtime shutdown and this change is not going to prevent that.

@zcbenz you are right on the Worker, so I've dropped the Worker changes. For the record, the #4506 trace unwinds from the detached thread's ~_State_impl into ~Worker, so ~CommandEncoder had already synchronized fine. The crash was the late cudaEventDestroy throwing, and non-throwing destructors cover that.

I think we are good now and hopefully the fixes can then unlock the the new release on mlx-swift. @zcbenz thanks for the support, appreciated.

@aleroot aleroot changed the title Fix CUDA termination crash via deterministic worker teardown and lazy stream evaluation Do not throw from CUDA destructors and avoid implicit default streams in eval/compile Sep 28, 2026
@zcbenz
zcbenz force-pushed the threads_and_strems_4506 branch from ed20ef1 to 2bba7fa Compare September 28, 2026 09:13
@zcbenz
zcbenz force-pushed the threads_and_strems_4506 branch from 2bba7fa to 0fc5521 Compare September 28, 2026 09:14
@zcbenz
zcbenz merged commit 9eb3e3a into ml-explore:main Sep 28, 2026
29 checks passed
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.

[BUG] threads and streams issues

2 participants