Source code downloaded via the debuggers sourceRequest creates lots of duplicate open files across debug sessions

[DanTup]

If I debug into a source file that is downloaded from the debugger (via sourceRequest) and then stop the debug session and repeat the same thing again, VS Code opens a second copy of the source (even though the name/contents are identical).

After a few debug sessions, I end up with this:

As far as I can tell, nothing is different about these files (besides the sourceReference) so I would expect VS Code to reuse the same existing file the next time they're required (I actually thought this is what it did in the past, though I'm not certain).

[isidorn]

We use the sourceReference in the URI of the file, so in order to have this de-duplicated you would have to provide the same sourceReference. We need to put it in the uri since we latter need to get in when we are providing the content.
I do not see how VS Code could improve this.

[DanTup]

@isidorn my understanding is that sourceReference is just an integer with no meaning across debug sessions - I can't think of a reliable way that a debug adapter could produce the same sourceReference for any given source file being downloaded from the debugger. We already provide a name/path for the Source too - could that not be used for uniqueness?

Or, could downloaded source files be closed when a debug session ends (or another is opened with the same name/path)?

Otherwise, do you have any ideas for how a debug adapter could handle this?

[isidorn]

We could close all debug sources once a debug session ends I guess, but they will still be in history.

We have uniqueness with the name, but we need the reference in the URI so we can get the content of that uri later on. Not for uniqueness.
Debug adapter could have a map of source path -> reference and to try to reuse references whenever possible.

[DanTup]

We could close all debug sources once a debug session ends I guess, but they will still be in history.

If these files were opened in "preview" mode, would that prevent this? It feels like these files make no sense to be open (or for a user to try to re-open) after the session ends, as the sourceReference (as I understand it) is never intended to live past the end of the debug session.

Debug adapter could have a map of source path -> reference and to try to reuse references whenever possible.

The problem is that it's across debug sessions, so this would require the DA persist this mapping somewhere, which feels a bit odd.

[roblourens]

It feels like these files make no sense to be open (or for a user to try to re-open) after the session ends, as the sourceReference (as I understand it) is never intended to live past the end of the debug session.

I agree. @TylerLeonhardt do you know whether there is a way to prevent some files from ending up in quickopen history, or clear them when the debug session is done?

[TylerLeonhardt]

As far as I know, something like this isn't supported, right @bpasero?

[bpasero]

@TylerLeonhardt yup, IHistoryService#removeFromHistory.

[connor4312]

I'll take this. Some notes:

• There is a new setting to auto-close sources after a sessions ends if a user wants that debug: close read-only tabs on end debug session #199898
• It makes sense to remove the files from history (or avoid them being added there in the first place)
• I actually do really want to deduplicate files across multiple sessions. If the same-named launch config produces another source with the same reference and path as one we've seen, I want to treat them as the same. This would make JavaScript debugging much nicer, as not infrequently I end up needing to debug into node internals or other files that aren't on disk, and it's always annoying to have my breakpoints jump to a 'new' identical file.

[DanTup]

I actually do really want to deduplicate files across multiple sessions. If the same-named launch config produces another source with the same reference and path as one we've seen, I want to treat them as the same.

What do you mean by "source reference" here? Most of the sourceReference values are just increasing integers and will depend on the order that sources are found in the debugger, so I think there should be some static identifiers that are not tied to that.

[connor4312]

Most of the sourceReference values are just increasing integers and will depend on the order that sources are found in the debugger, so I think there should be some static identifiers that are not tied to that.

I think the combination of identical sourceReference and path are sufficient to say that a given source is the same. In the javascript debugger, the sourceReference is actually a hash of source information (with increments on collisions) with the goal that breakpoints set in those sources work across multiple debug sessions. This does in fact work, but editor handling remains messy.

[DanTup]

I think the combination of identical sourceReference and path are sufficient to say that a given source is the same. In the javascript debugger, the sourceReference is actually a hash of source information

path is not a required field and in some cases our sources don't have paths (that's why we're downloading them from the VM - they might have URIs and be generated content).

While it may be good for JS because it's already using hashes, I think it would be much better to leave this up to the debug adapter and use a field specifically for this (for example allowing using string URIs or similar). The DAP spec also says sourceReferences are only valid for a session so reusing this would change that, but a new field would keep that the same.

[connor4312]

Yes, my thought was that if a path was not defined, we would not try to do any equivalency logic.

for example allowing using string URIs or similar

Would this be an opportunity to provide more info than what can be encoded in a path? For a non-zero sourceReference, it's already assumed the path does not exist on disk, so a DA could encode arbitrary information into the path (given it's at least moderately user-readable)

And yes, we provide some clarity in DAP

[DanTup]

Would this be an opportunity to provide more info than what can be encoded in a path?

Yeah - maybe this ties into microsoft/debug-adapter-protocol#444. VS Code supports URIs but it's not documented very clearly, and there's this weird pathFormat that apparently allows the client to indicate random strings as formats (having "etc." in a spec like this isn't very helpful 😄):

microsoft/debug-adapter-protocol#444 (comment)

That said, maybe a URI is not enough. For example I might have:

• dart:core/foo.dart - this is fixed, however it may change with your SDK version (so it's not really unique to some specific source)
• dart-generated:foo/bar.dart - this might be generated code based on the users files, so them making changes to their files may invalidate it

So I feel like we really need an explicit flag to say whether a source might be stable - and we shouldn't couple the identifier to what the user sees (for example I might want to see dart:core/foo.dartin the UI, but I really want my unique identifier to bedart-v1.2.3/dart:core/foo.dart` to ensure a diff SDK version doesn't try to reuse a file of the same URI).