提交 d9ab6eb1 编写于 作者: D Dustin Campbell

Fix Code Model race for real this time

I thought I'd fixed this subtle race with PR #8607. After all, the check-in tests were passing and it passed locally on my machine.
However, it was still failing consistently on the build server. Finally, I noticed that the only test passes that were failing were
on x86 *Release* (insert sound of forehead being smacked). After compiling Release, I could reproduce the problem locally.

It turns out that this was a race with GC. In a previous fix, when adding an element to FileCodeModel's element cache, we would first
check to see if the element was already in the cache. If it was in the cache, we'd remove it before adding it. However, the test to
see if the element was in the cache called `CleanableWeakComHandleTable.TryGetValue(TKey key, out TValue value)`, which would only return
true if `key` was in the cache and `value` was not set to null. In essence, it's defined like so:

```C#
public bool TryGetValue(TKey key, out TValue value)
{
    this.AssertIsForeground();

    WeakComHandle<TValue, TValue> handle;
    if (_table.TryGetValue(key, out handle))
    {
        value = handle.ComAggregateObject;
        return value != null;
    }

    value = null;
    return false;
}
```

However, when running in Release, the GC can run a bit more aggresively and `handle.ComAggregatedObject` (a weak reference) could easily be null.

The fix is to add a simpler `CleanableWeakComHandleTable.ContainsKey(TKey key)` and use that instead.

```C#
public bool ContainsKey(TKey key)
{
    this.AssertIsForeground();

    return _table.ContainsKey(key);
}
```
上级 533d09ec
......@@ -212,6 +212,13 @@ public TValue Remove(TKey key)
return null;
}
public bool ContainsKey(TKey key)
{
this.AssertIsForeground();
return _table.ContainsKey(key);
}
public bool TryGetValue(TKey key, out TValue value)
{
this.AssertIsForeground();
......
......@@ -199,8 +199,7 @@ internal void OnCodeElementCreated(SyntaxNodeKey nodeKey, EnvDTE.CodeElement ele
// If we're creating an element with the same node key as an element that's already in the table, just remove
// the old element. The old element will continue to function but the new element will replace it in the cache.
EnvDTE.CodeElement oldElement;
if (_codeElementTable.TryGetValue(nodeKey, out oldElement))
if (_codeElementTable.ContainsKey(nodeKey))
{
_codeElementTable.Remove(nodeKey);
}
......
......@@ -3845,22 +3845,7 @@ class C$$
Await TestElement(code,
Sub(state, codeClass)
For i = 1 To 100
Dim initialDocument = state.FileCodeModelObject.GetDocument()
Dim variable As EnvDTE.CodeVariable
Try
variable = codeClass.AddVariable("x", "System.Int32")
Catch ex As InvalidOperationException
Assert.False(True,
$"Failed to add variable in loop iteration {i}!
Exception: {ex.Message}
Document text:
{initialDocument.GetTextAsync(CancellationToken.None).Result}")
Throw ex
End Try
Dim variable = codeClass.AddVariable("x", "System.Int32")
' Now, delete the variable that we just added.
Dim startPoint = variable.StartPoint
......
......@@ -3154,22 +3154,7 @@ End Class
Await TestElement(code,
Sub(state, codeClass)
For i = 1 To 100
Dim initialDocument = state.FileCodeModelObject.GetDocument()
Dim variable As EnvDTE.CodeVariable
Try
variable = codeClass.AddVariable("x", "System.Int32")
Catch ex As InvalidOperationException
Assert.False(True,
$"Failed to add variable in loop iteration {i}!
Exception: {ex.Message}
Document text:
{initialDocument.GetTextAsync(CancellationToken.None).Result}")
Throw ex
End Try
Dim variable = codeClass.AddVariable("x", "System.Int32")
' Now, delete the variable that we just added.
Dim startPoint = variable.StartPoint
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册