Merge pull request #1970 from Hosch250/parserCancellationBug

Fix cancellation bug.  Don't request reparse on startup--request it w…

Author: retailcoder

RetailCoder.VBE/API/ParserState.cs

@@ -78,7 +78,7 @@ public void Initialize(VBE vbe)
         public void Parse()
         {
             // blocking call
-            _parser.Parse();
+            _parser.Parse(new System.Threading.CancellationTokenSource());
         }
 
         /// <summary>

RetailCoder.VBE/App.cs

@@ -11,7 +11,6 @@
 using Rubberduck.UI.Command.MenuItems;
 using System;
 using System.Globalization;
-using System.Threading.Tasks;
 using System.Windows.Forms;
 
 namespace Rubberduck
@@ -141,7 +140,6 @@ public void Startup()
             _appMenus.Initialize();
             _hooks.HookHotkeys(); // need to hook hotkeys before we localize menus, to correctly display ShortcutTexts
             _appMenus.Localize();
-            Task.Delay(1000).ContinueWith(t => UiDispatcher.Invoke(() => _parser.State.OnParseRequested(this)));
             UpdateLoggingLevel();
         }
 

Rubberduck.Parsing/Symbols/DeclarationSymbolsListener.cs

@@ -9,7 +9,6 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Runtime.InteropServices;
-using Antlr4.Runtime.Misc;
 
 namespace Rubberduck.Parsing.Symbols
 {
@@ -801,8 +800,8 @@ public override void ExitPrivateTypeDeclaration(VBAParser.PrivateTypeDeclaration
         {
             _parentDeclaration = _moduleDeclaration;
         }
-        
-        public void AddUdtDeclaration(VBAParser.UdtDeclarationContext udtDeclaration, Accessibility accessibility, ParserRuleContext context)
+
+        private void AddUdtDeclaration(VBAParser.UdtDeclarationContext udtDeclaration, Accessibility accessibility, ParserRuleContext context)
         {
             var identifier = Identifier.GetName(udtDeclaration.untypedIdentifier());
             var identifierSelection = Identifier.GetNameSelection(udtDeclaration.untypedIdentifier());

Rubberduck.Parsing/VBA/RubberduckParser.cs

@@ -20,9 +20,7 @@ namespace Rubberduck.Parsing.VBA
     public class RubberduckParser : IRubberduckParser
     {
         public RubberduckParserState State { get { return _state; } }
-
-        private CancellationTokenSource _central = new CancellationTokenSource();
-        private CancellationTokenSource _resolverTokenSource; // linked to _central later
+        
         private readonly ConcurrentDictionary<VBComponent, Tuple<Task, CancellationTokenSource>> _currentTasks =
             new ConcurrentDictionary<VBComponent, Tuple<Task, CancellationTokenSource>>();
 
@@ -43,7 +41,6 @@ public RubberduckParser(
             Func<IVBAPreprocessor> preprocessorFactory,
             IEnumerable<ICustomDeclarationLoader> customDeclarationLoaders)
         {
-            _resolverTokenSource = CancellationTokenSource.CreateLinkedTokenSource(_central.Token);
             _vbe = vbe;
             _state = state;
             _attributeParser = attributeParser;
@@ -53,12 +50,17 @@ public RubberduckParser(
             state.ParseRequest += ReparseRequested;
         }
 
+        // Do not access this from anywhere but ReparseRequested.
+        // ReparseRequested needs to have a reference to all the cancellation tokens,
+        // but the cancelees need to use their own token.
+        private readonly List<CancellationTokenSource> _cancellationTokens = new List<CancellationTokenSource> {new CancellationTokenSource()};
+
         private void ReparseRequested(object sender, ParseRequestEventArgs e)
         {
             if (e.IsFullReparseRequest)
             {
                 Cancel();
-                Task.Run(() => ParseAll());
+                Task.Run(() => ParseAll(_cancellationTokens[0]));
             }
             else
             {
@@ -70,14 +72,14 @@ private void ReparseRequested(object sender, ParseRequestEventArgs e)
                     SyncComReferences(State.Projects);
                     AddBuiltInDeclarations();
 
-                    if (_resolverTokenSource.IsCancellationRequested || _central.IsCancellationRequested)
+                    if (_cancellationTokens[0].IsCancellationRequested)
                     {
                         return;
                     }
 
-                    ParseAsync(e.Component, CancellationToken.None).Wait();
+                    ParseAsync(e.Component, _cancellationTokens[0]).Wait();
 
-                    if (_resolverTokenSource.IsCancellationRequested || _central.IsCancellationRequested)
+                    if (_cancellationTokens[0].IsCancellationRequested)
                     {
                         return;
                     }
@@ -93,7 +95,7 @@ private void ReparseRequested(object sender, ParseRequestEventArgs e)
                     if (State.Status < ParserState.Error)
                     {
                         State.SetStatusAndFireStateChanged(ParserState.ResolvedDeclarations);
-                        ResolveReferencesAsync();
+                        ResolveReferencesAsync(_cancellationTokens[0].Token);
                     }
                 });
             }
@@ -102,7 +104,7 @@ private void ReparseRequested(object sender, ParseRequestEventArgs e)
         /// <summary>
         /// For the use of tests only
         /// </summary>
-        public void Parse()
+        public void Parse(CancellationTokenSource token)
         {
             if (State.Projects.Count == 0)
             {
@@ -153,9 +155,9 @@ public void Parse()
                 var index = i;
                 parseTasks[i] = new Task(() =>
                 {
-                    ParseAsync(components[index], CancellationToken.None).Wait();
+                    ParseAsync(components[index], token).Wait(token.Token);
 
-                    if (_resolverTokenSource.IsCancellationRequested || _central.IsCancellationRequested)
+                    if (token.IsCancellationRequested)
                     {
                         return;
                     }
@@ -177,14 +179,14 @@ public void Parse()
             if (State.Status < ParserState.Error)
             {
                 State.SetStatusAndFireStateChanged(ParserState.ResolvedDeclarations);
-                Task.WaitAll(ResolveReferencesAsync());
+                Task.WaitAll(ResolveReferencesAsync(token.Token));
             }
         }
 
         /// <summary>
         /// Starts parsing all components of all unprotected VBProjects associated with the VBE-Instance passed to the constructor of this parser instance.
         /// </summary>
-        private void ParseAll()
+        private void ParseAll(CancellationTokenSource token)
         {
             if (State.Projects.Count == 0)
             {
@@ -230,18 +232,13 @@ private void ParseAll()
             }
 
             var toParse = new List<VBComponent>();
-            var unchanged = new List<VBComponent>();
 
             foreach (var component in components)
             {
                 if (State.IsNewOrModified(component))
                 {
                     toParse.Add(component);
                 }
-                else
-                {
-                    unchanged.Add(component);
-                }
             }
 
             if (toParse.Count == 0)
@@ -272,6 +269,11 @@ private void ParseAll()
                 }
             }
 
+            if (token.IsCancellationRequested)
+            {
+                return;
+            }
+
             _projectDeclarations.Clear();
             State.ClearBuiltInReferences();
 
@@ -281,9 +283,9 @@ private void ParseAll()
                 var index = i;
                 parseTasks[i] = new Task(() =>
                 {
-                    ParseAsync(toParse[index], CancellationToken.None).Wait();
+                    ParseAsync(toParse[index], token).Wait(token.Token);
 
-                    if (_resolverTokenSource.IsCancellationRequested || _central.IsCancellationRequested)
+                    if (token.IsCancellationRequested)
                     {
                         return;
                     }
@@ -301,16 +303,21 @@ private void ParseAll()
                 parseTasks[i].Start();
             }
 
+            if (token.IsCancellationRequested)
+            {
+                return;
+            }
+
             Task.WaitAll(parseTasks);
 
             if (State.Status < ParserState.Error)
             {
                 State.SetStatusAndFireStateChanged(ParserState.ResolvedDeclarations);
-                ResolveReferencesAsync();
+                ResolveReferencesAsync(token.Token);
             }
         }
 
-        private Task[] ResolveReferencesAsync()
+        private Task[] ResolveReferencesAsync(CancellationToken token)
         {
             var finder = new DeclarationFinder(State.AllDeclarations, State.AllComments, State.AllAnnotations);
             var passes = new List<ICompilationPass>
@@ -327,7 +334,7 @@ private Task[] ResolveReferencesAsync()
             for (var index = 0; index < State.ParseTrees.Count; index++)
             {
                 var kvp = State.ParseTrees[index];
-                if (_resolverTokenSource.IsCancellationRequested || _central.IsCancellationRequested)
+                if (token.IsCancellationRequested)
                 {
                     return new Task[0];
                 }
@@ -337,7 +344,7 @@ private Task[] ResolveReferencesAsync()
                     State.SetModuleState(kvp.Key.Component, ParserState.ResolvingReferences);
 
                     ResolveReferences(finder, kvp.Key.Component, kvp.Value);
-                });
+                }, token);
             }
 
             return tasks;
@@ -489,49 +496,44 @@ private void UnloadComReference(Reference reference, IReadOnlyList<VBProject> pr
             }
         }
 
-        private Task ParseAsync(VBComponent component, CancellationToken token, TokenStreamRewriter rewriter = null)
+        private Task ParseAsync(VBComponent component, CancellationTokenSource token, TokenStreamRewriter rewriter = null)
         {
             State.ClearStateCache(component);
 
-            var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(_central.Token, token);
-
-            var task = new Task(() => ParseAsyncInternal(component, linkedTokenSource.Token, rewriter));
-            _currentTasks.TryAdd(component, Tuple.Create(task, linkedTokenSource));
+            var task = new Task(() => ParseAsyncInternal(component, token.Token, rewriter));
+            _currentTasks.TryAdd(component, Tuple.Create(task, token));
 
             Tuple<Task, CancellationTokenSource> removedTask;
-            task.ContinueWith(t => _currentTasks.TryRemove(component, out removedTask)); // default also executes on cancel
+            task.ContinueWith(t => _currentTasks.TryRemove(component, out removedTask), token.Token); // default also executes on cancel
             // See http://stackoverflow.com/questions/6800705/why-is-taskscheduler-current-the-default-taskscheduler
             task.Start(TaskScheduler.Default);
             return task;
         }
 
         public void Cancel(VBComponent component = null)
         {
-            lock (_central)
-                lock (_resolverTokenSource)
+            lock (_cancellationTokens[0])
+            {
+                if (component == null)
                 {
-                    if (component == null)
-                    {
-                        _central.Cancel(false);
+                    _cancellationTokens[0].Cancel();
 
-                        _central.Dispose();
-                        _central = new CancellationTokenSource();
-                        _resolverTokenSource = CancellationTokenSource.CreateLinkedTokenSource(_central.Token);
-                    }
-                    else
+                    _cancellationTokens[0].Dispose();
+                    _cancellationTokens.Add(new CancellationTokenSource());
+                    _cancellationTokens.RemoveAt(0);
+                }
+                else
+                {
+                    _cancellationTokens[0].Cancel();
+                    
+                    Tuple<Task, CancellationTokenSource> result;
+                    if (_currentTasks.TryGetValue(component, out result))
                     {
-                        _resolverTokenSource.Cancel(false);
-                        _resolverTokenSource.Dispose();
-
-                        _resolverTokenSource = CancellationTokenSource.CreateLinkedTokenSource(_central.Token);
-                        Tuple<Task, CancellationTokenSource> result;
-                        if (_currentTasks.TryGetValue(component, out result))
-                        {
-                            result.Item2.Cancel(false);
-                            result.Item2.Dispose();
-                        }
+                        result.Item2.Cancel();
+                        result.Item2.Dispose();
                     }
                 }
+            }
         }
 
         private void ParseAsyncInternal(VBComponent component, CancellationToken token, TokenStreamRewriter rewriter = null)
@@ -569,6 +571,7 @@ private void ResolveDeclarations(VBComponent component, IParseTree tree)
 
             var qualifiedModuleName = new QualifiedModuleName(component);
 
+            var stopwatch = Stopwatch.StartNew();
             try
             {
                 var project = component.Collection.Parent;
@@ -581,6 +584,7 @@ private void ResolveDeclarations(VBComponent component, IParseTree tree)
                     State.AddDeclaration(projectDeclaration);
                 }
                 Logger.Debug("Creating declarations for module {0}.", qualifiedModuleName.Name);
+                
                 var declarationsListener = new DeclarationSymbolsListener(State, qualifiedModuleName, component.Type, State.GetModuleAnnotations(component), State.GetModuleAttributes(component), projectDeclaration);
                 ParseTreeWalker.Default.Walk(declarationsListener, tree);
                 foreach (var createdDeclaration in declarationsListener.CreatedDeclarations)
@@ -593,6 +597,8 @@ private void ResolveDeclarations(VBComponent component, IParseTree tree)
                 Logger.Error(exception, "Exception thrown acquiring declarations for '{0}' (thread {1}).", component.Name, Thread.CurrentThread.ManagedThreadId);
                 State.SetModuleState(component, ParserState.ResolverError);
             }
+            stopwatch.Stop();
+            Logger.Debug("{0}ms to resolve declarations for component {1}", stopwatch.ElapsedMilliseconds, component.Name);
         }
 
         private Declaration CreateProjectDeclaration(QualifiedModuleName projectQualifiedName, VBProject project)
@@ -654,15 +660,10 @@ public void Dispose()
         {
             State.ParseRequest -= ReparseRequested;
 
-            if (_central != null)
-            {
-                //_central.Cancel();
-                _central.Dispose();
-            }
-
-            if (_resolverTokenSource != null)
+            if (_cancellationTokens[0] != null)
             {
-                _resolverTokenSource.Dispose();
+                _cancellationTokens[0].Cancel();
+                _cancellationTokens[0].Dispose();
             }
         }
     }

RubberduckTests/Binding/IndexDefaultBindingTests.cs

@@ -4,6 +4,7 @@
 using Rubberduck.Parsing.VBA;
 using RubberduckTests.Mocks;
 using System.Linq;
+using System.Threading;
 using Moq;
 using Rubberduck.Parsing;
 
@@ -86,7 +87,7 @@ End Property
         private static RubberduckParserState Parse(Moq.Mock<VBE> vbe)
         {
             var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock<ISinks>().Object));
-            parser.Parse();
+            parser.Parse(new CancellationTokenSource());
             if (parser.State.Status != ParserState.Ready)
             {
                 Assert.Inconclusive("Parser state should be 'Ready', but returns '{0}'.", parser.State.Status);

RubberduckTests/Binding/MemberAccessDefaultBindingTests.cs

@@ -5,6 +5,7 @@
 using RubberduckTests.Mocks;
 using System;
 using System.Linq;
+using System.Threading;
 using Moq;
 using Rubberduck.Parsing;
 
@@ -95,7 +96,7 @@ public void LExpressionIsEnum()
         private static RubberduckParserState Parse(Moq.Mock<VBE> vbe)
         {
             var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock<ISinks>().Object));
-            parser.Parse();
+            parser.Parse(new CancellationTokenSource());
             if (parser.State.Status != ParserState.Ready)
             {
                 Assert.Inconclusive("Parser state should be 'Ready', but returns '{0}'.", parser.State.Status);

RubberduckTests/Binding/MemberAccessProcedurePointerBindingTests.cs

@@ -7,6 +7,7 @@
 using System.Linq;
 using Moq;
 using Rubberduck.Parsing;
+using System.Threading;
 
 namespace RubberduckTests.Binding
 {
@@ -40,7 +41,7 @@ public void ProceduralModuleWithAccessibleMember()
             private static RubberduckParserState Parse(Moq.Mock<VBE> vbe)
             {
                 var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object, new Mock<ISinks>().Object));
-                parser.Parse();
+                parser.Parse(new CancellationTokenSource());
                 if (parser.State.Status != ParserState.Ready)
                 {
                     Assert.Inconclusive("Parser state should be 'Ready', but returns '{0}'.", parser.State.Status);