diff --git a/src/System.Management.Automation/engine/debugger/Breakpoint.cs b/src/System.Management.Automation/engine/debugger/Breakpoint.cs index e51d79f4afa..c4b699bd152 100644 --- a/src/System.Management.Automation/engine/debugger/Breakpoint.cs +++ b/src/System.Management.Automation/engine/debugger/Breakpoint.cs @@ -482,9 +482,6 @@ internal bool TrySetBreakpoint(string scriptFile, FunctionContext functionContex { Diagnostics.Assert(SequencePointIndex == -1, "shouldn't be trying to set on a pending breakpoint"); - if (!scriptFile.Equals(this.Script, StringComparison.OrdinalIgnoreCase)) - return false; - // A quick check to see if the breakpoint is within the scriptblock. bool couldBeInNestedScriptBlock; var scriptBlock = functionContext._scriptBlock; @@ -595,11 +592,11 @@ internal override bool RemoveSelf(ScriptDebugger debugger) var boundBreakPoints = debugger.GetBoundBreakpoints(this.SequencePoints); if (boundBreakPoints != null) { - Diagnostics.Assert(boundBreakPoints.Contains(this), + Diagnostics.Assert(boundBreakPoints[this.SequencePointIndex].Contains(this), "If we set _scriptBlock, we should have also added the breakpoint to the bound breakpoint list"); - boundBreakPoints.Remove(this); + boundBreakPoints[this.SequencePointIndex].Remove(this); - if (boundBreakPoints.All(breakpoint => breakpoint.SequencePointIndex != this.SequencePointIndex)) + if (boundBreakPoints[this.SequencePointIndex].All(breakpoint => breakpoint.SequencePointIndex != this.SequencePointIndex)) { // No other line breakpoints are at the same sequence point, so disable the breakpoint so // we don't go looking for breakpoints the next time we hit the sequence point. diff --git a/src/System.Management.Automation/engine/debugger/debugger.cs b/src/System.Management.Automation/engine/debugger/debugger.cs index 2d154bec0f2..16595c6b088 100644 --- a/src/System.Management.Automation/engine/debugger/debugger.cs +++ b/src/System.Management.Automation/engine/debugger/debugger.cs @@ -971,7 +971,8 @@ internal ScriptDebugger(ExecutionContext context) _context = context; _inBreakpoint = false; _idToBreakpoint = new ConcurrentDictionary(); - _pendingBreakpoints = new ConcurrentDictionary(); + // The string key is function context file path. The int key is sequencePoint index. + _pendingBreakpoints = new ConcurrentDictionary>(); _boundBreakpoints = new ConcurrentDictionary>>(StringComparer.OrdinalIgnoreCase); _commandBreakpoints = new ConcurrentDictionary(); _variableBreakpoints = new ConcurrentDictionary>(StringComparer.OrdinalIgnoreCase); @@ -1174,7 +1175,7 @@ internal void EnterScriptFunction(FunctionContext functionContext) private void SetupBreakpoints(FunctionContext functionContext) { var scriptDebugData = _mapScriptToBreakpoints.GetValue(functionContext._sequencePoints, - _ => Tuple.Create(new List(), + _ => Tuple.Create(new Dictionary>(), new BitArray(functionContext._sequencePoints.Length))); functionContext._boundBreakpoints = scriptDebugData.Item1; functionContext._breakPoints = scriptDebugData.Item2; @@ -1254,10 +1255,19 @@ private CommandBreakpoint AddCommandBreakpoint(CommandBreakpoint breakpoint) private LineBreakpoint AddLineBreakpoint(LineBreakpoint breakpoint) { AddBreakpointCommon(breakpoint); - _pendingBreakpoints[breakpoint.Id] = breakpoint; + AddPendingBreakpoint(breakpoint); + return breakpoint; } + private void AddPendingBreakpoint(LineBreakpoint breakpoint) + { + _pendingBreakpoints.AddOrUpdate( + breakpoint.Script, + new ConcurrentDictionary { [breakpoint.Id] = breakpoint }, + (_, dictionary) => { dictionary.TryAdd(breakpoint.Id, breakpoint); return dictionary; }); + } + private void AddNewBreakpoint(Breakpoint breakpoint) { LineBreakpoint lineBreakpoint = breakpoint as LineBreakpoint; @@ -1310,13 +1320,9 @@ private void UpdateBreakpoints(FunctionContext functionContext) return; } - foreach ((int breakpointId, LineBreakpoint item) in _pendingBreakpoints) + if (_pendingBreakpoints.TryGetValue(functionContext._file, out var dictionary) && !dictionary.IsEmpty) { - if (item.IsScriptBreakpoint && item.Script.Equals(functionContext._file, StringComparison.OrdinalIgnoreCase)) - { - SetPendingBreakpoints(functionContext); - break; - } + SetPendingBreakpoints(functionContext); } } } @@ -1342,7 +1348,11 @@ internal bool RemoveCommandBreakpoint(CommandBreakpoint breakpoint) => internal bool RemoveLineBreakpoint(LineBreakpoint breakpoint) { - bool removed = _pendingBreakpoints.Remove(breakpoint.Id, out _); + bool removed = false; + if (_pendingBreakpoints.TryGetValue(breakpoint.Script, out var dictionary)) + { + removed = dictionary.Remove(breakpoint.Id, out _); + } Tuple> value; if (_boundBreakpoints.TryGetValue(breakpoint.Script, out value)) @@ -1360,8 +1370,8 @@ internal bool RemoveLineBreakpoint(LineBreakpoint breakpoint) // The bit array is used to detect if a breakpoint is set or not for a given scriptblock. This bit array // is checked when hitting sequence points. Enabling/disabling a line breakpoint is as simple as flipping // the bit. - private readonly ConditionalWeakTable, BitArray>> _mapScriptToBreakpoints = - new ConditionalWeakTable, BitArray>>(); + private readonly ConditionalWeakTable>, BitArray>> _mapScriptToBreakpoints = + new ConditionalWeakTable>, BitArray>>(); /// /// Checks for command breakpoints. @@ -1462,9 +1472,9 @@ internal void TriggerVariableBreakpoints(List breakpoints) // Return the line breakpoints bound in a specific script block (used when a sequence point // is hit, to find which breakpoints are set on that sequence point.) - internal List GetBoundBreakpoints(IScriptExtent[] sequencePoints) + internal Dictionary> GetBoundBreakpoints(IScriptExtent[] sequencePoints) { - Tuple, BitArray> tuple; + Tuple>, BitArray> tuple; if (_mapScriptToBreakpoints.TryGetValue(sequencePoints, out tuple)) { return tuple.Item1; @@ -1550,16 +1560,25 @@ internal void OnSequencePointHit(FunctionContext functionContext) { if (functionContext._breakPoints[functionContext._currentSequencePointIndex]) { - var breakpoints = (from breakpoint in functionContext._boundBreakpoints - where - breakpoint.SequencePointIndex == functionContext._currentSequencePointIndex && - breakpoint.Enabled - select breakpoint).ToList(); - - breakpoints = TriggerBreakpoints(breakpoints); - if (breakpoints.Count > 0) + if (functionContext._boundBreakpoints.TryGetValue(functionContext._currentSequencePointIndex, out var sequencePointBreakpoints)) { - StopOnSequencePoint(functionContext, breakpoints); + var enabledBreakpoints = new List(); + foreach (Breakpoint breakpoint in sequencePointBreakpoints) + { + if (breakpoint.Enabled) + { + enabledBreakpoints.Add(breakpoint); + } + } + + if (enabledBreakpoints.Count > 0) + { + enabledBreakpoints = TriggerBreakpoints(enabledBreakpoints); + if (enabledBreakpoints.Count > 0) + { + StopOnSequencePoint(functionContext, enabledBreakpoints); + } + } } } } @@ -1673,7 +1692,7 @@ internal void Clear() } private readonly ExecutionContext _context; - private ConcurrentDictionary _pendingBreakpoints; + private readonly ConcurrentDictionary> _pendingBreakpoints; private readonly ConcurrentDictionary>> _boundBreakpoints; private readonly ConcurrentDictionary _commandBreakpoints; private readonly ConcurrentDictionary> _variableBreakpoints; @@ -1981,16 +2000,20 @@ private void UnbindBoundBreakpoints(List boundBreakpoints) foreach (var breakpoint in boundBreakpoints) { // Also remove unbound breakpoints from the script to breakpoint map. - Tuple, BitArray> lineBreakTuple; + Tuple>, BitArray> lineBreakTuple; if (_mapScriptToBreakpoints.TryGetValue(breakpoint.SequencePoints, out lineBreakTuple)) { - lineBreakTuple.Item1.Remove(breakpoint); + if (lineBreakTuple.Item1.TryGetValue(breakpoint.SequencePointIndex, out var lineBreakList)) + { + lineBreakList.Remove(breakpoint); + } } breakpoint.SequencePoints = null; breakpoint.SequencePointIndex = -1; breakpoint.BreakpointBitArray = null; - _pendingBreakpoints[breakpoint.Id] = breakpoint; + + AddPendingBreakpoint(breakpoint); } boundBreakpoints.Clear(); @@ -1998,23 +2021,24 @@ private void UnbindBoundBreakpoints(List boundBreakpoints) private void SetPendingBreakpoints(FunctionContext functionContext) { - if (_pendingBreakpoints.IsEmpty) - return; - - var newPendingBreakpoints = new Dictionary(); var currentScriptFile = functionContext._file; // If we're not in a file, we can't have any line breakpoints. if (currentScriptFile == null) return; + if (!_pendingBreakpoints.TryGetValue(currentScriptFile, out var breakpoints) || breakpoints.IsEmpty) + { + return; + } + // Normally we register a script file when the script is run or the module is imported, // but if there weren't any breakpoints when the script was run and the script was dotted, // we will end up here with pending breakpoints, but we won't have cached the list of // breakpoints in the script. RegisterScriptFile(currentScriptFile, functionContext.CurrentPosition.StartScriptPosition.GetFullScript()); - Tuple, BitArray> tuple; + Tuple>, BitArray> tuple; if (!_mapScriptToBreakpoints.TryGetValue(functionContext._sequencePoints, out tuple)) { Diagnostics.Assert(false, "If the script block is still alive, the entry should not be collected."); @@ -2022,7 +2046,7 @@ private void SetPendingBreakpoints(FunctionContext functionContext) Diagnostics.Assert(tuple.Item1 == functionContext._boundBreakpoints, "What's up?"); - foreach ((int breakpointId, LineBreakpoint breakpoint) in _pendingBreakpoints) + foreach ((int breakpointId, LineBreakpoint breakpoint) in breakpoints) { bool bound = false; if (breakpoint.TrySetBreakpoint(currentScriptFile, functionContext)) @@ -2033,21 +2057,32 @@ private void SetPendingBreakpoints(FunctionContext functionContext) } bound = true; - tuple.Item1.Add(breakpoint); + if (tuple.Item1.TryGetValue(breakpoint.SequencePointIndex, out var list)) + { + list.Add(breakpoint); + } + else + { + tuple.Item1.Add(breakpoint.SequencePointIndex, new List { breakpoint }); + } + // We need to keep track of any breakpoints that are bound in each script because they may // need to be rebound if the script changes. var boundBreakpoints = _boundBreakpoints[currentScriptFile].Item2; boundBreakpoints[breakpoint.Id] = breakpoint; } - if (!bound) + if (bound) { - newPendingBreakpoints.Add(breakpoint.Id, breakpoint); + breakpoints.TryRemove(breakpointId, out _); } } - _pendingBreakpoints = new ConcurrentDictionary(newPendingBreakpoints); + // Here could check if all breakpoints for the current functionContext were bound, but because there is no atomic + // api for conditional removal we either need to lock, or do some trickery that has possibility of race conditions. + // Instead we keep the item in the dictionary with 0 breakpoint count. This should not be a big issue, + // because it is single entry per file that had breakpoints, so there won't be thousands of files in a session. } private void StopOnSequencePoint(FunctionContext functionContext, List breakpoints) diff --git a/src/System.Management.Automation/engine/parser/Compiler.cs b/src/System.Management.Automation/engine/parser/Compiler.cs index 99b38a3fa06..3fac9de74f6 100644 --- a/src/System.Management.Automation/engine/parser/Compiler.cs +++ b/src/System.Management.Automation/engine/parser/Compiler.cs @@ -782,7 +782,7 @@ internal class FunctionContext internal ExecutionContext _executionContext; internal Pipe _outputPipe; internal BitArray _breakPoints; - internal List _boundBreakpoints; + internal Dictionary> _boundBreakpoints; internal int _currentSequencePointIndex; internal MutableTuple _localsTuple; internal List[], Type[]>> _traps = new List[], Type[]>>();