Skip to content

Conversation

@sgmunn
Copy link
Member

@sgmunn sgmunn commented Dec 4, 2019

Fixes VSTS 1026591
SigTerm signal in Mono.Debugging.dll!Mono.Debugging.Client.DebuggerSession::get_Breakpoints+9

slock is used throughout DebuggerSession, however in this case we only need to guard against
multiple threads assigning an instance of BreakpointStore. Also, since BreakpointStore is thread-safe
we can remove the additional locks around it where we lock on enumeration of breakpointStore.

This fixes the case where one thread dispatches an OnExit (for example) which tries communicate
with the debuggee and waits on data from the socket and another thread attempts to clean up breakpoints.

@sgmunn
Copy link
Member Author

sgmunn commented Dec 10, 2019

@slluis this does not pass the tests - mono/monodevelop#9431

Fixes VSTS 1026591
SigTerm signal in Mono.Debugging.dll!Mono.Debugging.Client.DebuggerSession::get_Breakpoints+9

`slock` is used throughout DebuggerSession, however in this case we only need to guard against
multiple threads assigning an instance of BreakpointStore. Also, since BreakpointStore is thread-safe
we can remove the additional locks around it where we lock on enumeration of `breakpointStore`.

This fixes the case where one thread dispatches an OnExit (for example) which tries communicate
with the debuggee and waits on data from the socket and another thread attempts to clean up breakpoints.
We changed the locking semantics and the dispatch will no longer wait for the
current thread to release its lock when assigning the new breakpoint store
the session is created. Fix timing issue of assigning breakpoints.
@jstedfast
Copy link
Member

The reason this was failing was because of a commit that made it so that vm.Resume() did not get called properly after SoftDebuggerSession received a VMStart event, so the vm would just hang.

@jstedfast
Copy link
Member

I've got a new build of mono/monodevelop#9431 going - if that passes the unit tests and DDRITs, then his should be safe to merge.

@jstedfast jstedfast self-assigned this Dec 12, 2019
@jstedfast
Copy link
Member

Tests are passing

@Therzok
Copy link
Contributor

Therzok commented Dec 13, 2019

nice!

@slluis slluis merged commit 281e01e into master Dec 13, 2019
@slluis slluis deleted the vsts-1026591 branch December 13, 2019 09:15
mauroa pushed a commit to mauroa/debugger-libs that referenced this pull request Sep 21, 2023
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.

7 participants