microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.53k stars 29.4k forks source link

Case mismatch in setBreakpoints requests in a single session #232088

Open nisargjhaveri opened 3 weeks ago

nisargjhaveri commented 3 weeks ago

Does this issue occur when all extensions are disabled?: Yes

Issue: In some cases, the setBreakpoints requests sent in the same session sends the path with different cases. Especially on case-insensitive file systems. This causes broken breakpoint experience on some debugger. Currently tested with CodeLLDB and android-debug on Windows. Issue is fairly easy to repro and affects across machines and projects.

Debugging details: After some debugging, here is what I gathered.

The issue boils down to the two different paths taken by getRawSource. For a local file, if this.sources contains an entry for that URI, it directly returns source.raw. Otherwise, it returns name and path using canonical URI. https://github.com/microsoft/vscode/blob/45a9f8902795ca9a10f411db90f690f67843d88d/src/vs/workbench/contrib/debug/browser/debugSession.ts#L1507-L1515

Form what I saw this.sources is populated when resolving frames on a breakpoint hit. The debugger sends frames with raw source info. That info is stored and used as is once available.

The issue arises when the canonical path and the raw source from debugger doesn't match. e.g. canonical path is <path>/Native-Lib.cpp but the debugger reported <path>/native-lib.cpp. This means that until the this.sources was populated for this Uri, we sent <path>/Native-Lib.cpp when setting breakpoints. And later, we send <path>/native-lib.cpp. This mismatch causes lldb to treat the paths differently and duplicate breakpoints.

Potential solution:

roblourens commented 2 weeks ago

Yeah, there is a larger issue here, also related: https://github.com/microsoft/vscode/issues/210854. I'll look at your PR, but the implications are a little confusing and it might be a partial fix.

Why does the debugger return a different casing? Which is "correct"?

nisargjhaveri commented 2 weeks ago

At the time of initialization or early in debugging, the raw Source from debugger is not present for most breakpoints. At this point, we create a new raw source using Source.getEncodedDebugData. See the below snippet.

https://github.com/microsoft/vscode/blob/848d216ba9a8e0194043826cb359d37f972547d1/src/vs/workbench/contrib/debug/browser/debugSession.ts#L1512-L1513

This generally returns the canonicalized path which the editor decides.

The debug adapter also sends Sources in different contexts as I can see, one of them being stack trace when a breakpoint hits. All the sources sent by the debug adapter are then cached in this.sources with the canonical uri as the key. Later when setting a breakpoint, this cached raw source from debug adapter is used as is.

https://github.com/microsoft/vscode/blob/848d216ba9a8e0194043826cb359d37f972547d1/src/vs/workbench/contrib/debug/browser/debugSession.ts#L1508-L1511

The issue occurs when the initial source created using Source.getEncodedDebugData and the Sources sent by debug adapter has different casing for path. This can happen for various reasons on a case-insensitive file systems as different systems have different assumptions and different ways of handing the case-insensitivity.

This results in different setBreakpoint requests to have differently cased path for the same file on the editor side. This creates issues with some debuggers as noted in the issue.

The "correct" casing can be debated, but I'd assume it is safe to send the canonical path which matches the file system. In any case, for this particular issue, "correct" might not be that important, but "consistent" is, since the issue only occurs because of difference in path handling in editor and debug adapter.

nisargjhaveri commented 5 days ago

@roblourens Any thoughts on this? I'd really appreciate if you can have a look at the issue and the associated PR.