This is only called from a single place after we took them out from collection that linked to the current file. So we are sure those belong to the current file. So this is just unnecessary overhead.

How would you want this solved? This should happen rarely so I might lock here. Or just keep it as is and don't clean up the dictionary of files.

I don't fully understand the problem we're trying to solve here, but if you/@PaulHigin is able to explain it to me, I can try to weigh in

This is pending breakpoints:

_pendingBreakpoints = new ConcurrentDictionary<string, ConcurrentDictionary<int, LineBreakpoint>>()

Pending breakpoints is a dictionary keyed by filepath, that contains a dictionary keyed by sequence point. When all pending breakpoints were bound, it would be nice to remove the key from the _pendingBreakpoints dictionary.

Something like this:

if (_pendingBreakpoints.TryGetValue(currentScriptFile, out var bpsInThisScript) && bpsInThisScript.IsEmpty) {
    _pendingBreakpoints.TryRemoveValue(currentScriptFile, out _);
}

Unfortunately the first line is not atomic, so there is a race condition between the first line and the second. If someone added breakpoint right after we checked the count, in theory we could lose breakpoints.

This seems like a rare condition and can be solved in few ways.

What I did here is that I simply leave the key in the dictionary. This means 1 extra string + empty concurrent dictionary is left in memory, for each file that had breakpoints. I am guessing there are rarely more than 100 distinct files with breakpoints per powershell session, so this seems okay-ish. But still dirty.

The race condition seems very rare, and simply checking if we removed a dictionary in which an item was added and they try to merge it back in might reduce the possibility of removing a breakpoint even further. We would add another race condition, but the possibility of timing both of them exactly right seems infinitely small. Something like this:

if (_pendingBreakpoints.TryGetValue(currentScriptFile, out var bpsInThisScript) && bpsInThisScript.IsEmpty) {
    if (_pendingBreakpoints.TryRemoveValue(currentScriptFile, out var removedBps) && !removedBps.IsEmpty) {
        // someone added a breakpoint after we counted but before we removed
        // merge it back into _pendingBreakpoints
        // this would happen extremely rarely
    }
}

The race condition needs to be better defined here. How is setting a breakpoint subject to a race here, via API? PowerShell scripts run on a single thread.

That is why I was asking how you want this solved because I don't know enough about the interaction of pending breakpoints and code execution. Maybe there is no way that pending breakpoints collection would be changed while this code is running, because it all runs on a single thread, or maybe adding a breakpoint in vscode UI calls into the PowerShell process and sets the breakpoint from a different thread.

I just assumed it is the latter, which is why ConcurrentDictionary was used in the original code and also in the new code.