Skip to content

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Aug 18, 2025

This replaces deterministic fails with intermittent hangs and crashes for backward compatibility.

Contributes to #118741

@Copilot Copilot AI review requested due to automatic review settings August 18, 2025 01:02
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR disables a release assert for managed C++ applications that attempt to execute managed code after .NET runtime thread state has been destroyed. The change provides backward compatibility by replacing deterministic failures with intermittent hangs and crashes for mixed-mode binaries.

Key changes:

  • Conditionally disables a runtime assert based on whether IJW (It Just Works) mixed-mode binaries have been loaded
  • Adds tracking of IJW binary loading state through a global flag
  • Provides a new public method to check if any IJW binaries have been loaded

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/coreclr/vm/ceemain.cpp Conditionally disables assert for thread re-initialization when IJW binaries are present
src/coreclr/vm/ceeload.h Declares new static method to check IJW loading state
src/coreclr/vm/ceeload.cpp Implements IJW tracking logic and exposes check method

Copy link
Contributor

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mangod9
Copy link
Member

mangod9 commented Aug 18, 2025

Is it not better to deterministically fail rather than indeterminate behavior. If this was "undefined" can we check if there is any workaround available here?

@VSadov
Copy link
Member

VSadov commented Aug 18, 2025

Is it not better to deterministically fail rather than indeterminate behavior. If this was "undefined" can we check if there is any workaround available here?

A thread has just told us "here is the last chance to clean up" and we have destroyed all the state for the thread in the runtime and now it wants to run managed code again. There is no good solution to this. We cannot recover the previous state. We can set up some new state to a degree that the thread can run "simple" managed code, but hard to tell what will happen in all possible scenarios. Also, there will not be another cleanup callback, so the new state will leak and, when the thread truly exits, may become corrupted. (which is probably ok, if the process is terminating).

Yes, deterministic fail would be better, if there was no common scenario that triggers this, but now we have discovered one such scenario.

I was thinking of maybe forcing a thread that tries to "re-attach" into some kind of "no-GC" mode, but it may just trade some potential issues for others.

The mitigation seems unfortunate, but narrowing it just for IJW case (which wants this at process exit) makes it less worrisome.

@jkotas
Copy link
Member Author

jkotas commented Aug 18, 2025

Is it not better to deterministically fail rather than indeterminate behavior. If this was "undefined" can we check if there is any workaround available here?

This check uncovered a long-standing bug in managed C/C++ compiler and libraries in basic use case. It is not clear whether and when C/C++ team will be able to fix it.

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 4236d91 into dotnet:main Aug 18, 2025
96 of 98 checks passed
@AaronRobinsonMSFT
Copy link
Member

/backport to release/9.0-staging

Copy link
Contributor

Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/17044005889

@mangod9
Copy link
Member

mangod9 commented Aug 18, 2025

/backport to release/9.0-staging

needs to be moved to 10-rc1 too?

@AaronRobinsonMSFT
Copy link
Member

/backport to release/9.0-staging

needs to be moved to 10-rc1 too?

Yes... I realized that on the bus in.

@AaronRobinsonMSFT
Copy link
Member

/backport to release/10.0-rc1

Copy link
Contributor

Started backporting to release/10.0-rc1: https://github.com/dotnet/runtime/actions/runs/17045414642

@AaronRobinsonMSFT
Copy link
Member

/backport to release/10.0

Copy link
Contributor

Started backporting to release/10.0: https://github.com/dotnet/runtime/actions/runs/17135493496

@jkotas jkotas deleted the ijw-workaround branch August 27, 2025 00:46
@github-actions github-actions bot locked and limited conversation to collaborators Sep 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants