Skip to content

Commit a31b0d5

Browse files
committed
Made setting module states honor cancellation in a threadsafe way.
1 parent 1b757b8 commit a31b0d5

File tree

2 files changed

+54
-27
lines changed

2 files changed

+54
-27
lines changed

Rubberduck.Parsing/VBA/ParseCoordinator.cs

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,12 @@ private void ReparseRequested(object sender, EventArgs e)
7878
if (!_isTestScope)
7979
{
8080
Cancel();
81-
Task.Run(() => ParseAll(sender, _cancellationTokens[0]));
81+
Task.Run(() => ParseAll(sender, _cancellationTokens[0].Token));
8282
}
8383
else
8484
{
8585
Cancel();
86-
ParseInternal(_cancellationTokens[0]);
86+
ParseInternal(_cancellationTokens[0].Token);
8787
}
8888
}
8989

@@ -108,7 +108,7 @@ private void Cancel(bool createNewTokenSource = true)
108108
public void Parse(CancellationTokenSource token)
109109
{
110110
SetSavedCancellationTokenSource(token);
111-
ParseInternal(token);
111+
ParseInternal(token.Token);
112112
}
113113

114114
private void SetSavedCancellationTokenSource(CancellationTokenSource token)
@@ -125,7 +125,7 @@ private void SetSavedCancellationTokenSource(CancellationTokenSource token)
125125
}
126126
}
127127

128-
private void ParseInternal(CancellationTokenSource token)
128+
private void ParseInternal(CancellationToken token)
129129
{
130130
State.RefreshProjects(_vbe);
131131

@@ -160,9 +160,9 @@ private void CleanUpComponentAttributes(List<IVBComponent> components)
160160
}
161161
}
162162

163-
private void ExecuteCommonParseActivities(List<IVBComponent> toParse, CancellationTokenSource token)
163+
private void ExecuteCommonParseActivities(List<IVBComponent> toParse, CancellationToken token)
164164
{
165-
SetModuleStates(toParse, ParserState.Pending);
165+
SetModuleStates(toParse, ParserState.Pending, token);
166166

167167
SyncComReferences(State.Projects);
168168
RefreshDeclarationFinder();
@@ -178,14 +178,14 @@ private void ExecuteCommonParseActivities(List<IVBComponent> toParse, Cancellati
178178
_projectDeclarations.Clear();
179179
State.ClearBuiltInReferences();
180180

181-
ParseComponents(toParse, token.Token);
181+
ParseComponents(toParse, token);
182182

183183
if (token.IsCancellationRequested || State.Status >= ParserState.Error)
184184
{
185185
return;
186186
}
187187

188-
ResolveAllDeclarations(toParse, token.Token);
188+
ResolveAllDeclarations(toParse, token);
189189
RefreshDeclarationFinder();
190190

191191
if (token.IsCancellationRequested || State.Status >= ParserState.Error)
@@ -200,7 +200,7 @@ private void ExecuteCommonParseActivities(List<IVBComponent> toParse, Cancellati
200200
return;
201201
}
202202

203-
ResolveAllReferences(token.Token);
203+
ResolveAllReferences(token);
204204

205205
if (token.IsCancellationRequested || State.Status >= ParserState.Error)
206206
{
@@ -215,19 +215,23 @@ private void RefreshDeclarationFinder()
215215
State.RefreshFinder(_hostApp);
216216
}
217217

218-
private void SetModuleStates(List<IVBComponent> components, ParserState parserState)
218+
private void SetModuleStates(List<IVBComponent> components, ParserState parserState, CancellationToken token)
219219
{
220220
var options = new ParallelOptions();
221+
options.CancellationToken = token;
221222
options.MaxDegreeOfParallelism = _maxDegreeOfModuleStateChangeParallelism;
222223

223-
Parallel.ForEach(components, options, component => State.SetModuleState(component, parserState, null, false));
224-
225-
State.EvaluateParserState();
224+
Parallel.ForEach(components, options, component => State.SetModuleState(component, parserState, token, null, false));
225+
226+
if (!token.IsCancellationRequested)
227+
{
228+
State.EvaluateParserState();
229+
}
226230
}
227231

228232
private void ParseComponents(List<IVBComponent> components, CancellationToken token)
229233
{
230-
SetModuleStates(components, ParserState.Parsing);
234+
SetModuleStates(components, ParserState.Parsing, token);
231235

232236
var options = new ParallelOptions();
233237
options.CancellationToken = token;
@@ -241,7 +245,7 @@ private void ParseComponents(List<IVBComponent> components, CancellationToken to
241245
{
242246
State.ClearStateCache(component);
243247
var finishedParseTask = FinishedParseComponentTask(component, token);
244-
ProcessComponentParseResults(component, finishedParseTask);
248+
ProcessComponentParseResults(component, finishedParseTask, token);
245249
}
246250
);
247251
}
@@ -286,19 +290,19 @@ private void ParseComponents(List<IVBComponent> components, CancellationToken to
286290
}
287291

288292

289-
private void ProcessComponentParseResults(IVBComponent component, Task<ComponentParseTask.ParseCompletionArgs> finishedParseTask)
293+
private void ProcessComponentParseResults(IVBComponent component, Task<ComponentParseTask.ParseCompletionArgs> finishedParseTask, CancellationToken token)
290294
{
291295
if (finishedParseTask.IsFaulted)
292296
{
293297
//In contrast to the situation in the success scenario, the overall parser state is reevaluated immediately.
294-
State.SetModuleState(component, ParserState.Error, finishedParseTask.Exception.InnerException as SyntaxErrorException);
298+
State.SetModuleState(component, ParserState.Error, token, finishedParseTask.Exception.InnerException as SyntaxErrorException);
295299
}
296300
else if (finishedParseTask.IsCompleted)
297301
{
298302
var result = finishedParseTask.Result;
299303
lock (State)
300304
{
301-
lock (component)
305+
lock (component)
302306
{
303307
State.SetModuleAttributes(component, result.Attributes);
304308
State.AddParseTree(component, result.ParseTree);
@@ -309,7 +313,7 @@ private void ProcessComponentParseResults(IVBComponent component, Task<Component
309313
// This really needs to go last
310314
//It does not reevaluate the overall parer state to avoid concurrent evaluation of all module states and for performance reasons.
311315
//The evaluation has to be triggered manually in the calling procedure.
312-
State.SetModuleState(component, ParserState.Parsed, null, false);
316+
State.SetModuleState(component, ParserState.Parsed, token, null, false); //Note that this is ok because locks allow re-entrancy.
313317
}
314318
}
315319
}
@@ -318,7 +322,7 @@ private void ProcessComponentParseResults(IVBComponent component, Task<Component
318322

319323
private void ResolveAllDeclarations(List<IVBComponent> components, CancellationToken token)
320324
{
321-
SetModuleStates(components, ParserState.ResolvingDeclarations);
325+
SetModuleStates(components, ParserState.ResolvingDeclarations, token);
322326

323327
var options = new ParallelOptions();
324328
options.CancellationToken = token;
@@ -331,7 +335,8 @@ private void ResolveAllDeclarations(List<IVBComponent> components, CancellationT
331335
{
332336
var qualifiedName = new QualifiedModuleName(component);
333337
ResolveDeclarations(qualifiedName.Component,
334-
State.ParseTrees.Find(s => s.Key == qualifiedName).Value);
338+
State.ParseTrees.Find(s => s.Key == qualifiedName).Value,
339+
token);
335340
}
336341
);
337342
}
@@ -346,7 +351,7 @@ private void ResolveAllDeclarations(List<IVBComponent> components, CancellationT
346351
}
347352

348353
private readonly ConcurrentDictionary<string, Declaration> _projectDeclarations = new ConcurrentDictionary<string, Declaration>();
349-
private void ResolveDeclarations(IVBComponent component, IParseTree tree)
354+
private void ResolveDeclarations(IVBComponent component, IParseTree tree, CancellationToken token)
350355
{
351356
if (component == null) { return; }
352357

@@ -376,7 +381,7 @@ private void ResolveDeclarations(IVBComponent component, IParseTree tree)
376381
catch (Exception exception)
377382
{
378383
Logger.Error(exception, "Exception thrown acquiring declarations for '{0}' (thread {1}).", component.Name, Thread.CurrentThread.ManagedThreadId);
379-
State.SetModuleState(component, ParserState.ResolverError);
384+
State.SetModuleState(component, ParserState.ResolverError, token);
380385
}
381386
stopwatch.Stop();
382387
Logger.Debug("{0}ms to resolve declarations for component {1}", stopwatch.ElapsedMilliseconds, component.Name);
@@ -409,7 +414,7 @@ private Declaration CreateProjectDeclaration(QualifiedModuleName projectQualifie
409414
private void ResolveAllReferences(CancellationToken token)
410415
{
411416
var components = State.ParseTrees.Select(kvp => kvp.Key.Component).ToList();
412-
SetModuleStates(components, ParserState.ResolvingReferences);
417+
SetModuleStates(components, ParserState.ResolvingReferences, token);
413418

414419
if (token.IsCancellationRequested)
415420
{
@@ -503,7 +508,7 @@ private void ResolveReferences(DeclarationFinder finder, QualifiedModuleName qua
503508
watch.ElapsedMilliseconds, Thread.CurrentThread.ManagedThreadId);
504509

505510
//Evaluation of the overall status has to be defered to allow processing of undeclared variables before setting the ready state.
506-
State.SetModuleState(qualifiedName.Component, ParserState.Ready, null, false);
511+
State.SetModuleState(qualifiedName.Component, ParserState.Ready, token, null, false);
507512
}
508513
catch (OperationCanceledException)
509514
{
@@ -512,7 +517,7 @@ private void ResolveReferences(DeclarationFinder finder, QualifiedModuleName qua
512517
catch (Exception exception)
513518
{
514519
Logger.Error(exception, "Exception thrown resolving '{0}' (thread {1}).", qualifiedName.Name, Thread.CurrentThread.ManagedThreadId);
515-
State.SetModuleState(qualifiedName.Component, ParserState.ResolverError);
520+
State.SetModuleState(qualifiedName.Component, ParserState.ResolverError, token);
516521
}
517522
}
518523
}
@@ -530,7 +535,7 @@ private void AddUndeclaredVariablesToDeclarations()
530535
/// <summary>
531536
/// Starts parsing all components of all unprotected VBProjects associated with the VBE-Instance passed to the constructor of this parser instance.
532537
/// </summary>
533-
private void ParseAll(object requestor, CancellationTokenSource token)
538+
private void ParseAll(object requestor, CancellationToken token)
534539
{
535540
State.RefreshProjects(_vbe);
536541

Rubberduck.Parsing/VBA/RubberduckParserState.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@ private void OnStateChanged(object requestor, ParserState state = ParserState.Pe
301301
}
302302
public event EventHandler<ParseProgressEventArgs> ModuleStateChanged;
303303

304+
//Never spawn new threads changing module states in the handler! This will cause deadlocks.
304305
private void OnModuleStateChanged(IVBComponent component, ParserState state, ParserState oldState)
305306
{
306307
var handler = ModuleStateChanged;
@@ -311,7 +312,27 @@ private void OnModuleStateChanged(IVBComponent component, ParserState state, Par
311312
}
312313
}
313314

315+
314316
public void SetModuleState(IVBComponent component, ParserState state, SyntaxErrorException parserError = null, bool evaluateOverallState = true)
317+
{
318+
lock (component)
319+
{
320+
SetModuleStateInternal(component, state, parserError, evaluateOverallState);
321+
}
322+
}
323+
324+
public void SetModuleState(IVBComponent component, ParserState state, CancellationToken token, SyntaxErrorException parserError = null, bool evaluateOverallState = true)
325+
{
326+
lock (component)
327+
{
328+
if (!token.IsCancellationRequested)
329+
{
330+
SetModuleStateInternal(component, state, parserError, evaluateOverallState);
331+
}
332+
}
333+
}
334+
335+
public void SetModuleStateInternal(IVBComponent component, ParserState state, SyntaxErrorException parserError = null, bool evaluateOverallState = true)
315336
{
316337
if (AllUserDeclarations.Count > 0)
317338
{
@@ -357,6 +378,7 @@ public void SetModuleState(IVBComponent component, ParserState state, SyntaxErro
357378
}
358379
}
359380

381+
360382
public void EvaluateParserState()
361383
{
362384
lock (_statusLockObject) Status = OverallParserStateFromModuleStates();

0 commit comments

Comments
 (0)