Skip to content
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.

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

Release CordbUnmanagedThread instance from CordbProcess member #66910

Merged
merged 2 commits into from
Mar 22, 2022

Conversation

AaronRobinsonMSFT
Copy link
Member

Fixes #65357

The m_lastDispatchedIBEvent was not being cleared during shutdown, which was causing a memory leak assert to fire.

See

// Make sure we keep the thread alive while we're playing with it.
pUnmanagedThread->InternalAddRef();
LOG((LF_CORDB, LL_INFO10000, "CP::DUE: dispatching unmanaged event %d for thread 0x%x\n",
pUnmanagedEvent->m_currentDebugEvent.dwDebugEventCode, pUnmanagedThread->m_id));
m_dispatchingUnmanagedEvent = true;
// Add/Remove a reference which is scoped to the time that m_lastDispatchedIBEvent
// is set to pUnmanagedEvent (it is an interior pointer)
// see DevDiv issue 818301 for more details
if(m_lastDispatchedIBEvent != NULL)
{
m_lastDispatchedIBEvent->m_owner->InternalRelease();
m_lastDispatchedIBEvent = NULL;
}
pUnmanagedThread->InternalAddRef();
m_lastDispatchedIBEvent = pUnmanagedEvent;
pUnmanagedEvent->SetState(CUES_Dispatched);

I added the clearing in the NeuterChildren() method because it seemed like the best place. Suggestions on more appropriate places is welcome.

/cc @dotnet/dotnet-diag

The m_lastDispatchedIBEvent was not be cleared during shutdown,
  which was causing a memory leak assert to fire.
Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com>
@AaronRobinsonMSFT
Copy link
Member Author

/cc @noahfalk @hoyosjs

Copy link
Member

@hoyosjs hoyosjs left a comment

Choose a reason for hiding this comment

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

Feels pretty safe - this is a flush, shutdown, or detach path. Might not mimic the best lifetime for the event being held but can't think of a better place to have it - you need it for other continue events and native event convoys. I couldn't find a case that wouldn't fall in this scenario as long as the debugger impl is checking the shutdown/detach path for interop.

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 7a6c808 into dotnet:main Mar 22, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the runtime_65357 branch March 22, 2022 19:00
thaystg pushed a commit to thaystg/runtime that referenced this pull request Mar 22, 2022
…otnet#66910)

* Release CordbUnmanagedThread from CordbProcess member

The m_lastDispatchedIBEvent was not being cleared during shutdown, which was causing a memory leak assert to fire.

Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com>
radekdoulik pushed a commit to radekdoulik/runtime that referenced this pull request Mar 30, 2022
…otnet#66910)

* Release CordbUnmanagedThread from CordbProcess member

The m_lastDispatchedIBEvent was not being cleared during shutdown, which was causing a memory leak assert to fire.

Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com>
radekdoulik pushed a commit to radekdoulik/runtime that referenced this pull request Mar 30, 2022
…otnet#66910)

* Release CordbUnmanagedThread from CordbProcess member

The m_lastDispatchedIBEvent was not being cleared during shutdown, which was causing a memory leak assert to fire.

Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com>
@ghost ghost locked as resolved and limited conversation to collaborators Apr 21, 2022
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.

corerun debugger fails on exit with "debugger type: mixed (.net core)"
3 participants