Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

[Local GC] Fix some interface violations#10817

Merged
jkotas merged 2 commits intodotnet:masterfrom
swgillespie:interface-violations
Apr 8, 2017
Merged

[Local GC] Fix some interface violations#10817
jkotas merged 2 commits intodotnet:masterfrom
swgillespie:interface-violations

Conversation

@swgillespie
Copy link

This PR fixes two linker errors that arise when building the GC standalone:

  1. A use of _DebugBreak in GCHeap::WaitUntilGCComplete, replaced with GCToOSInterface::DebugBreak
  2. A use of EEPOLICY_HANDLE_FATAL_ERROR in FATAL_GC_ERROR, replaced with GCToEEInterface::HandleFatalError.

cc @Maoni0 @adityamandaleeka @sergiy-k @jkotas

// taken by a suspended thread.
FreeBuildDebugBreak();
// Even in retail, stop in the debugger if available.
GCToOSInterface::DebugBreak();
Copy link
Member

Choose a reason for hiding this comment

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

It looks like __FreeBuildDebugBreak (used in release builds) checks whether CLRConfig::INTERNAL_BreakOnRetailAssert is set, and does nothing if it isn't. Is this change in behavior intentional?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, at least for now. In a future PR I'll be adding a mechanism by which we can query config values from the GC while build standalone, so we could choose do to that if we'd like. This is the only place in the GC that I found that honors BreakOnRetailAssert, though - all other places just call DebugBreak unconditionally and the process will die if there's no debugger attached.

Copy link
Member

Choose a reason for hiding this comment

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

This code is under DETECT_DEADLOCK ifdef. This ifdef does not seem to be turned on, so it should not matter much what we do here.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good.

@jkotas jkotas merged commit ea55c4a into dotnet:master Apr 8, 2017
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants