Fixes for thread-created and terminate/detach #558
Conversation
| string msgString = "msg"; | ||
| // In lldb, the format is ^error,message="" | ||
| // In gdb/vsdbg it is ^error,msg="" | ||
| if (_launchOptions.DebuggerMIMode == MIMode.Lldb) |
There was a problem hiding this comment.
Since this is a bug in LLDB that they might eventually fix, I might suggest instead Trying to fix 'msg', and if that property doesn't exist look for 'message'
| { | ||
| _callback.OnThreadStart(thread); |
There was a problem hiding this comment.
The one problem I can see with this is that in CollectThreadsInfo we will not just have the id, but we will also have the 'TargetId'. It looks like the 'TargetId' is what we expose to AD7. Here, we only have the Id. AD7 is not going to be happy if a thread's ID changes.
| ResultEventArgs result = (ResultEventArgs)args; | ||
| _callback.OnError(result.Results.FindString("msg")); | ||
| string msgString = "msg"; |
There was a problem hiding this comment.
I suggest TryFind "msg" first and then for "message" (if LLDB) so that this doesn't fail in case they ever "fix" the issue on the LLDB side.
|
@gregg-miskelly @paulmaybee I refactored the thread info block and am now reporting thread created and thread exited events. when we get them. |
| { | ||
| lock (_threadList) | ||
| ResultValue resVal = null; |
There was a problem hiding this comment.
You need a try/catch around this method now, as async void methods can never let exceptions leave
| @@ -46,6 +46,30 @@ internal class ThreadCache | |||
| private static uint s_targetId; | |||
| private const string c_defaultGroupId = "i1"; // gdb's default group id, also used for any process without group ids | |||
|
|
|||
| private List<DebuggedThread> deadThreads | |||
There was a problem hiding this comment.
d [](start = 37, length = 1)
nit: the convention in this code base is to capitalize the first letter of properties.
| string targetId = resVal.TryFindString("target-id"); | ||
|
|
||
| DebuggedThread thread = FindThread(threadId, out newThread); | ||
| thread.Alive = true; |
There was a problem hiding this comment.
thread.Alive = true; [](start = 12, length = 20)
Should this really be in CollectThreadInfo?
There was a problem hiding this comment.
@gregg-miskelly This was there already. I just moved it. I think its there because if thread info lists it, it is still alive.
There was a problem hiding this comment.
@gregg-miskelly This was there already. I just moved it. I think its there because if thread info lists it, it is still alive.
|
@gregg-miskelly Verified on android that break all, breakpoints and threads windows still work. |
1. Fix for Pause on OS X with lldb. LLDB does not break on attach so threads were not being sent to the UI. Fix was to send back messages when threads are created directly 2. fix for detach/terminate. With the change to do an internal break, it was sending -exec-interrupt, -exec-abort and then -exec-continue which was causing errors. added check in DoInternalBreakActions to only continue if we are not terminating or detaching. 3. Fix for ErrorEvent. on LLDB, the message format is ^error,message="" but in gdb it is ^error,msg="". Added a special case for it.
pieandcakes
force-pushed
the
users/piel/threadcreated
branch
from
ba8eac2
to
4b923dc
Compare
Fix for Pause on OS X with lldb. LLDB does not break on attach so
threads were not being sent to the UI. Fix was to send back messages when
threads are created directly
fix for detach/terminate. With the change to do an internal break, it
was sending -exec-interrupt, -exec-abort and then -exec-continue which was
causing errors. added check in DoInternalBreakActions to only continue if
we are not terminating or detaching.
Fix for ErrorEvent. on LLDB, the message format is ^error,message=""
but in gdb it is ^error,msg="". Added a special case for it.
@gregg-miskelly @paulmaybee @rajkumar42