Support multi-threading #129

LeszekSwirski · 2018-02-02T11:02:44Z

Fixes #36. Not everything works perfectly, in particular non-varobj local don't have a well-defined thread, but it's a good base to build on. I'm not 100% sure about the logic behind VAR_HANDLES_START, so the current array size increase may be overkills

Also includes some odds-and-ends fixing parsing and reference printing.

WebFreak001 · 2018-02-02T11:05:59Z

src/backend/mi_parse.ts

 			return [];
+		}


uh any reason for this change? Could you include some test what this is supposed to fix or what didn't work in the tests file please

I don't remember the exact command that caused it, but it was to do with an mi command returning {} followed by other data, where the other data was getting lost

I've added an (artificial) test, which I can confirm did fail before this patch.

WebFreak001 · 2018-02-02T11:50:58Z

ok while quickly going over the changes I didn't notice any major issues so I will do more extensive testing later with all supported debuggers (not sure if mago supports multithreading, but it's just a side product anyway to allow D people on windows to debug if they really want to).

I don't really know anything about async in js so I need to lookup if any thread safety issues can occur there or if it's just fancy promises.

Otherwise I don't know how I feel about all the missing semicolons and using ' for strings longer than 1 character yet lol

But this seems like a good and useful PR, thanks

LeszekSwirski · 2018-02-02T11:54:35Z

async in JS is effectively equivalent to promise then chains, there's still no actual parallelism or threading. I can fix the semicolons and 's (EDIT: Done.)

I've only tested with gdb so YMMV, single-threaded functionality shouldn't change with other compilers though I think.

LeszekSwirski · 2018-02-14T12:09:49Z

Gentle reminder.

WebFreak001 · 2018-02-19T19:40:23Z

sorry was kind of busy with other stuff and didn't really feel like reviewing something large, I can do it now

WebFreak001 · 2018-02-19T20:12:58Z

GDB

single threaded still works locally
multi threaded now works locally
single threaded still works over SSH
multi threaded now works over SSH
single threaded still works over gdbserver
multi threaded now works over gdbserver
single threaded still works when attaching
multi threaded now works when attaching
Exception:

TypeError: Cannot read property 'getThreads' of undefined
    at GDBDebugSession.threadsRequest (/tmp/code-debug/out/src/mibase.js:239:24)
    at GDBDebugSession.dispatchRequest (/tmp/code-debug/node_modules/vscode-debugadapter/lib/debugSession.js:398:22)
    at GDBDebugSession._handleData (/tmp/code-debug/node_modules/vscode-debugadapter/lib/protocol.js:97:38)
    at Socket.inStream.on (/tmp/code-debug/node_modules/vscode-debugadapter/lib/protocol.js:18:44)
    at emitOne (events.js:96:13)
    at Socket.emit (events.js:191:7)
    at readableAddChunk (_stream_readable.js:178:18)
    at Socket.Readable.push (_stream_readable.js:136:10)
    at Pipe.onread (net.js:560:20): Error: TypeError: Cannot read property 'getThreads' of undefined
    at GDBDebugSession.threadsRequest (/tmp/code-debug/out/src/mibase.js:239:24)
    at GDBDebugSession.dispatchRequest (/tmp/code-debug/node_modules/vscode-debugadapter/lib/debugSession.js:398:22)
    at GDBDebugSession._handleData (/tmp/code-debug/node_modules/vscode-debugadapter/lib/protocol.js:97:38)
    at Socket.inStream.on (/tmp/code-debug/node_modules/vscode-debugadapter/lib/protocol.js:18:44)
    at emitOne (events.js:96:13)
    at Socket.emit (events.js:191:7)
    at readableAddChunk (_stream_readable.js:178:18)
    at Socket.Readable.push (_stream_readable.js:136:10)
    at Pipe.onread (net.js:560:20)
    at file:///opt/visual-studio-code/resources/app/out/vs/workbench/workbench.main.js:2644:530
    at Object.v [as _notify] (file:///opt/visual-studio-code/resources/app/out/vs/workbench/workbench.main.js:140:302)
    at Object.enter (file:///opt/visual-studio-code/resources/app/out/vs/workbench/workbench.main.js:143:568)
    at n.Class.derive._oncancel._run (file:///opt/visual-studio-code/resources/app/out/vs/workbench/workbench.main.js:144:866)
    at n.Class.derive._oncancel._error (file:///opt/visual-studio-code/resources/app/out/vs/workbench/workbench.main.js:144:383)
    at file:///opt/visual-studio-code/resources/app/out/vs/workbench/workbench.main.js:2223:975
    at _.e.dispatch (file:///opt/visual-studio-code/resources/app/out/vs/workbench/workbench.main.js:2225:291)
    at _.e.handleData (file:///opt/visual-studio-code/resources/app/out/vs/workbench/workbench.main.js:2224:815)
    at Socket.<anonymous> (file:///opt/visual-studio-code/resources/app/out/vs/workbench/workbench.main.js:2223:835)
    at emitOne (events.js:96:13)

LLDB

single threaded still works locally
multi threaded now works locally.
minor issue: thread names say undefined
single threaded still works over SSH (breakpoints don't work)
multi threaded now works over SSH (breakpoints don't work)
single threaded still works when attaching
multi threaded now works when attaching

Mago-MI
~~- [ ] single threaded still works locally~~ (can't test because I can't run a VM because I disabled AMD-V in my bios because it locks my CPU after some random amount of time)
I don't think testing anything else here makes sense, because it is very limited

But I supposed most of these issues were present already, I am going to check for completeness though. The only thing I see when testing here right now is therefore that undefined in the thread name of lldb.

WebFreak001 · 2018-02-19T20:18:03Z

ok so the Thread exception when it fails to connect is new, but gdbserver still doesn't work.

I did find one issue though: breakpoints show as verified in the broken lldb ssh connection now, they were unverified like this before

WebFreak001 · 2018-02-19T20:20:36Z

I can merge this as it is now because it is a good start, but I would prefer it if there were some // TODO comments where I just commented what behaviour changed (notably the exceptions always being verified, I saw there were some changes in exception mapping) and the undefined label for threads in lldb

LeszekSwirski · 2018-02-21T11:54:09Z

Thanks for the review, I've added some defensive ifs and TODOs. Regarding the breakpoints being verified, I think there's some semantic difference between breakpoints being verified, and being enabled. Since I get the "done" response back from lldb, I assume they're verified as far as lldb is concerned (modulo lldb mi bugs), but they might not actually be enabled because of various reasons. Regardless, there's a new callback-based breakpoint API coming, so that might do a better job of maintaining breakpoint state.

…names

WebFreak001 · 2018-02-21T17:12:34Z

src/mibase.ts

@@ -256,13 +258,24 @@ export class MI2DebugSession extends DebugSession {
 	}

 	protected threadsRequest(response: DebugProtocol.ThreadsResponse): void {
+		if (!this.miDebugger) {
+			this.sendResponse(response);


hm this should also return so the next call doesn't crash

Not sure why it only happens for this function but this fixes the error in the console.

WebFreak001 reviewed Feb 2, 2018

View reviewed changes

LeszekSwirski force-pushed the multi-threading branch from 0bd44f1 to 16ca7a9 Compare February 2, 2018 11:42

LeszekSwirski force-pushed the multi-threading branch from 16ca7a9 to e44a590 Compare February 2, 2018 12:13

LeszekSwirski added 2 commits February 2, 2018 12:17

Support multi-threading

ab0a3bb

Add a test

596674f

LeszekSwirski force-pushed the multi-threading branch from e44a590 to 596674f Compare February 2, 2018 12:17

Fix crash in threadsRequest with no miDebugger, and undefined thread …

a7b89c3

…names

LeszekSwirski force-pushed the multi-threading branch from 7e04ff5 to a7b89c3 Compare February 21, 2018 16:35

WebFreak001 reviewed Feb 21, 2018

View reviewed changes

Return if miDebugger is not set in threadsRequest

8af4b28

Not sure why it only happens for this function but this fixes the error in the console.

WebFreak001 merged commit bcbbda1 into WebFreak001:master Feb 21, 2018

LeszekSwirski deleted the multi-threading branch February 21, 2018 17:29

WebFreak001 mentioned this pull request Mar 8, 2018

breakpoints are not verified when remote debugging over ssh #135

Open

i-ky mentioned this pull request Jul 29, 2020

Update multithreaded support status #236

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support multi-threading #129

Support multi-threading #129

Uh oh!

LeszekSwirski commented Feb 2, 2018

Uh oh!

WebFreak001 Feb 2, 2018

Uh oh!

LeszekSwirski Feb 2, 2018

Uh oh!

LeszekSwirski Feb 2, 2018

Uh oh!

WebFreak001 commented Feb 2, 2018

Uh oh!

LeszekSwirski commented Feb 2, 2018 •

edited

Loading

Uh oh!

LeszekSwirski commented Feb 14, 2018

Uh oh!

WebFreak001 commented Feb 19, 2018

Uh oh!

WebFreak001 commented Feb 19, 2018

Uh oh!

WebFreak001 commented Feb 19, 2018

Uh oh!

WebFreak001 commented Feb 19, 2018 •

edited

Loading

Uh oh!

LeszekSwirski commented Feb 21, 2018

Uh oh!

WebFreak001 Feb 21, 2018

Uh oh!

Uh oh!

Support multi-threading #129

Support multi-threading #129

Uh oh!

Conversation

LeszekSwirski commented Feb 2, 2018

Uh oh!

WebFreak001 Feb 2, 2018

Choose a reason for hiding this comment

Uh oh!

LeszekSwirski Feb 2, 2018

Choose a reason for hiding this comment

Uh oh!

LeszekSwirski Feb 2, 2018

Choose a reason for hiding this comment

Uh oh!

WebFreak001 commented Feb 2, 2018

Uh oh!

LeszekSwirski commented Feb 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LeszekSwirski commented Feb 14, 2018

Uh oh!

WebFreak001 commented Feb 19, 2018

Uh oh!

WebFreak001 commented Feb 19, 2018

Uh oh!

WebFreak001 commented Feb 19, 2018

Uh oh!

WebFreak001 commented Feb 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LeszekSwirski commented Feb 21, 2018

Uh oh!

WebFreak001 Feb 21, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LeszekSwirski commented Feb 2, 2018 •

edited

Loading

WebFreak001 commented Feb 19, 2018 •

edited

Loading