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 into from
Apr 8, 2017
Merged

[Local GC] Fix some interface violations #10817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
edf8bd0
[Local GC] Fix some interface violations
swgillespie Apr 7, 2017
63ed79e
Repair the sample
swgillespie Apr 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/gc/env/gcenv.base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ inline HRESULT HRESULT_FROM_WIN32(unsigned long x)
#define E_UNEXPECTED 0x8000FFFF
#define E_NOTIMPL 0x80004001
#define E_INVALIDARG 0x80070057
#define COR_E_EXECUTIONENGINE 0x80131506

#define NOERROR 0x0
#define ERROR_TIMEOUT 1460
Expand Down
8 changes: 2 additions & 6 deletions src/gc/gcee.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,12 +408,8 @@ uint32_t GCHeap::WaitUntilGCComplete(bool bConsiderGCStart)
dwWaitResult = WaitForGCEvent->Wait(DETECT_DEADLOCK_TIMEOUT, FALSE );

if (dwWaitResult == WAIT_TIMEOUT) {
// Even in retail, stop in the debugger if available. Ideally, the
// following would use DebugBreak, but debspew.h makes this a null
// macro in retail. Note that in debug, we don't use the debspew.h
// macros because these take a critical section that may have been
// taken by a suspended thread.
FreeBuildDebugBreak();
// Even in retail, stop in the debugger if available.
GCToOSInterface::DebugBreak();
Copy link
Copy Markdown
Member

@adityamandaleeka adityamandaleeka Apr 8, 2017

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
Copy Markdown
Author

@swgillespie swgillespie Apr 8, 2017

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
Copy Markdown
Member

@jkotas jkotas Apr 8, 2017

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
Copy Markdown
Member

@adityamandaleeka adityamandaleeka Apr 8, 2017

Choose a reason for hiding this comment

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

Sounds good.

goto BlockAgain;
}

Expand Down
2 changes: 1 addition & 1 deletion src/gc/gcpriv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ inline void FATAL_GC_ERROR()
GCToOSInterface::DebugBreak();
#endif // DACCESS_COMPILE
_ASSERTE(!"Fatal Error in GC.");
EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE);
GCToEEInterface::HandleFatalError(COR_E_EXECUTIONENGINE);
}

#ifdef _MSC_VER
Expand Down