puremourning
commented
4 years ago Thanks for this detailed report. I did start working on writing some tests for this.
Closed sgoth closed 4 years ago
puremourning
commented
4 years ago Thanks for this detailed report. I did start working on writing some tests for this.
puremourning
commented
4 years ago I haven't forgotten about this... just need to find a block of time to write all the tests to work out the right behaviour.
puremourning
commented
4 years ago In addition to this, we may want to change vimspector not to 'aggressively' request threads (per https://github.com/puremourning/vimspector/issues/223#issuecomment-698843210).
Probably what vscode does (and therefore the de facto requirement) is to start populating the threads window from received thread events, but only request threads request on a stopped event.
In that case, we will have a thread ID, from a thread event (so pause request is possible), but we don't need to request all threads at that time.
Further, we probably need to start indicating the "current" thread in the threads window as a few requests actually do depend on the 'current' thread. We should add some way to "switch" the current thread in the UI too.
so, quite a lot of stuff needs to be done to make threads work properly. Just noting this here for reference when this all gets tested and fixed up properly.
puremourning
commented
4 years ago Actually, no. This is what the spec says:
Supporting threads
Whenever the generic debugger receives a stopped or a thread event, the development tool requests all threads that exist at that point in time. Thread events are optional, but a debug adapter can send them to force the development tool to update the threads UI dynamically even when not in a stopped state. If a debug adapter decides not to emit Thread events, the thread UI in the development tool will only update if a stopped event is received.
After a successful launch or attach, the development tool requests the baseline of currently existing threads with the threads request and then starts to listen for thread events to detect new or terminated threads. Even if a debug adapter does not support multiple threads, it must implement the threads request and return a single (dummy) thread. The thread id must be used in all requests which refer to a thread, e.g. stacktrace, pause, continue, next, stepIn, and stepOut.
So what vimspector is doing about "aggressively" requesting threads is 100% correct and the only bug is that we're not requesting threads on every thread event (only the first one). This should be easy to fix.
sgoth
commented
4 years ago Thanks for tackling this. Just fyi, I do still work with the change you proposed in https://gitter.im/vimspector/Lobby?at=5f8991b4c990bb1c39108ff7 and have not had any issues i'd attribute to it.
puremourning
commented
4 years ago @seezer would you mind giving #297 a spin? I have drastically improved vimspector thread handling.
sgoth
commented
4 years ago Sorry was rather busy lately.. Just tried current master (ff4acb1) and the issue here is definitely resolved. Thanks!
Fyi i got a red error message flashed "Protocol error: Server returned no threads" in the startup phase of the debugger. Worked without issues though. Perhaps querying the threads once a bit too early here? Total non issue but i figured you might be interested.
Log lines around the 'User Msg':
2020-11-25 16:27:48,400 - DEBUG - Message received: {'type': 'event', 'event': 'output', 'body': {'category': 'telemetry', 'output': 'VS/Diagnostics/Debugger/Launch', 'data': {'VS.Diagnostics.Debugger.ImplementationName': 'Microsoft.MIDebugEngine', 'VS.Diagnostics.Debugger.EngineVersion': '16.5.10325.1', 'VS.Diagnostics.Debugger.HostVersion': '16.5.10325.1', 'VS.Diagnostics.Debugger.AdapterId': 'cppdbg', 'VS.Diagnostics.Debugger.Launch.Duration': 610, 'VS.Diagnostics.Debugger.Launch.IsCoreDump': False, 'VS.Diagnostics.Debugger.VisualizerFileUsed': True, 'VS.Diagnostics.Debugger.SourceFileMappings': 0, 'VS.Diagnostics.Debugger.MIMode': None}}, 'seq': 3}
2020-11-25 16:27:48,465 - DEBUG - Message received: {'type': 'response', 'request_seq': 1, 'success': True, 'command': 'launch', 'body': {}, 'seq': 4}
2020-11-25 16:27:48,465 - DEBUG - Message received: {'type': 'event', 'event': 'initialized', 'body': {}, 'seq': 5}
2020-11-25 16:27:48,465 - DEBUG - Sending Message: {"command": "setFunctionBreakpoints", "arguments": {"breakpoints": []}, "seq": 3, "type": "request"}
2020-11-25 16:27:48,498 - DEBUG - Message received: {'type': 'response', 'request_seq': 3, 'success': True, 'command': 'setFunctionBreakpoints', 'body': {'breakpoints': []}, 'seq': 6}
2020-11-25 16:27:48,498 - DEBUG - Breakpoints at this point: {}
2020-11-25 16:27:48,498 - DEBUG - Sending Message: {"command": "configurationDone", "seq": 4, "type": "request"}
2020-11-25 16:27:48,521 - DEBUG - Message received: {'type': 'response', 'request_seq': 4, 'success': True, 'command': 'configurationDone', 'body': {}, 'seq': 7}
2020-11-25 16:27:48,521 - DEBUG - Sending Message: {"command": "threads", "seq": 5, "type": "request"}
2020-11-25 16:27:48,521 - DEBUG - Message received: {'type': 'event', 'event': 'output', 'body': {'category': 'stdout', 'output': '=thread-group-added,id="i1"\nGNU gdb (GDB) 10.1\nCopyright (C) 2020 Free Software Foundation, Inc.\nLicense GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>\nThis is free software: you are free to change and redistribute it.\nThere is NO WARRANTY, to the extent permitted by law.\nType "show copying" and "show warranty" for details.\nThis GDB was configured as "x86_64-pc-linux-gnu".\nType "show configuration" for configuration details.\nFor bug reporting instructions, please see:\n<https://www.gnu.org/software/gdb/bugs/>.\nFind the GDB manual and other documentation resources online at:\n <http://www.gnu.org/software/gdb/documentation/>.\n\nFor help, type "help".\nType "apropos word" to search for commands related to "word".\n=cmd-param-changed,param="follow-fork-mode",value="child"\nWarning: Debuggee TargetArchitecture not detected, assuming x86_64.\n=cmd-param-changed,param="pagination",value="off"\n'}, 'seq': 8}
2020-11-25 16:27:48,548 - DEBUG - Message received: {'type': 'response', 'request_seq': 5, 'success': True, 'command': 'threads', 'body': {'threads': []}, 'seq': 9}
2020-11-25 16:27:48,548 - INFO - User Msg: Protocol error: Server returned no threads
2020-11-25 16:27:48,561 - DEBUG - Message received: {'type': 'event', 'event': 'thread', 'body': {'reason': 'started', 'threadId': 980121}, 'seq': 10}
2020-11-25 16:27:48,561 - DEBUG - Sending Message: {"command": "threads", "seq": 6, "type": "request"}
2020-11-25 16:27:48,583 - DEBUG - Message received: {'type': 'event', 'event': 'output', 'body': {'category': 'stdout', 'output': 'Stopped due to shared library event (no libraries added or removed)\n'}, 'seq': 11}
2020-11-25 16:27:48,584 - DEBUG - Message received: {'type': 'response', 'request_seq': 6, 'success': True, 'command': 'threads', 'body': {'threads': [{'id': 980121, 'name': 'cassandra'}]}, 'seq': 12}
2020-11-25 16:27:48,584 - DEBUG - Sending Message: {"command": "stackTrace", "arguments": {"threadId": 980121}, "seq": 7, "type": "request"}
2020-11-25 16:27:48,602 - DEBUG - Message received: {'type': 'event', 'event': 'output', 'body': {'category': 'console', 'output': "Loaded '/lib64/ld-linux-x86-64.so.2'. Symbols loaded.\n"}, 'seq': 14}
2020-11-25 16:27:48,602 - DEBUG - Message received: {'type': 'response', 'request_seq': 7, 'success': True, 'command': 'stackTrace', 'body': {'stackFrames': [], 'totalFrames': 0}, 'seq': 13}
2020-11-25 16:27:53,618 - DEBUG - Message received: {'type': 'event', 'event': 'output', 'body': {'category': 'stdout', 'output': '[Thread debugging using libthread_db enabled]\n'}, 'seq': 15}
2020-11-25 16:27:53,619 - DEBUG - Message received: {'type': 'event', 'event': 'output', 'body': {'category': 'stdout', 'output': 'Using host libthread_db library "/usr/lib/libthread_db.so.1".\n'}, 'seq': 16}Protocol error: Server returned no threads
Yes, it seems that many servers have an interesting relationship with the specification, namely that they seem to think it's ok to ignore it.
You were actually getting this error before, the only change is that it's now red :)
sgoth
commented
4 years ago Haha yeah ok. I like red anyway. :)
puremourning
commented
4 years ago Actually, let me check that vimspector really is sending threads request at the right time.
puremourning
commented
4 years ago Yep, I even have a passive-aggressive comment in the code:
def _LoadThreadsIfReady( self ):
# NOTE: You might think we should only load threads on a stopped event,
# but the spec is clear:
#
# After a successful launch or attach the development tool requests the
# baseline of currently existing threads with the threads request and
# then starts to listen for thread events to detect new or terminated
# threads.
#
# Of course, specs are basically guidelines. MS's own cpptools simply
# doesn't respond top threads request when attaching via gdbserver. At
# least it would apear that way.
#
Describe the bug
Threads in StackView window are updated correctly when a breakpoint is hit or when continuing. Hitting 'pause' does show the correct stack for the selected thread but doesn't update the thread list. Threads created/destroyed in the meantime are missing/still there respectively.
Minimal reproduction
vim -Nu /path/to/vimspector/support/minimal_vimrc? [Yes]Using HUMAN keybinds here.
vim -Nu /path/to/vimspector/support/minimal_vimrcI do use miDebuggerArgs
set follow-fork-mode childin my project but it doesn't matter here. I state it to match my logs.Expected behaviour
When the debuggee is stopped by the debugger, current state is expected in all windows.
Actual behaviour
StackView shows the threads valid at time of last breakpoint even though Console shows that thread(s) have spawned. Pressing F5 immediately updates the threads correctly.
Same is true for the reverse, when threads go away.
Log shows an application running to a breakpoint with 3 threads active at that time. Then continues and spawns a bunch of new threads which are not visible on pause.
Environment
Version of Vimspector: (e.g. output of
git rev-parse HEADif cloned or the name of the tarball used to install otherwise) 78bec87f4ed058dfad6f8c33f487719db10f1bd3Output of
vim --versionornvim --versionwhich vimorwhich nvim::py3 print( __import__( 'sys' ).version )::py3 import vim::py3 import vimspector:Declaration
Additional information
Discussion on gitter: https://gitter.im/vimspector/Lobby?at=5f898a0beb82301c1a2cd099 Running the proposed potential fix from https://gitter.im/vimspector/Lobby?at=5f8991b4c990bb1c39108ff7 currently. Will report any issues with that here.