mirror of https://github.com/icsharpcode/ILSpy.git
Browse Source
The crash is a NullReferenceException in GetNodeByVisibleIndex, reached when a background decompile realizes a node's children while the UI thread is indexing the flattener. Eight ILSpyTreeNode.Decompile overrides call EnsureLazyChildren from that task; two wrapped it in Dispatcher.UIThread.Invoke, six did not, and one of the two lost its wrapper in the Avalonia port with no test noticing for a release cycle. A rule every call site has to remember is a rule that gets broken again, so EnsureLazyChildren marshals itself instead: SetOwner already named the owning thread, and now also carries the host's way onto it. A call already on the owner runs inline, so a blocking invoke cannot deadlock on itself and a nested load costs no further hop; an unowned tree is left unmarshalled, which keeps building a subtree on a worker and publishing it on the UI thread legal. The affinity check stays as the regression detector, but its fail-fast throw was worthless on its own: tree mutation happens inside callers that catch Exception, so the throw ended up rendered into the decompiled output and the run passed. The violation is now recorded before the throw, and an assembly-level NUnit test action fails the test that produced one - an assembly-level teardown failure is reported but leaves the exit code at zero. Assisted-by: Claude:claude-opus-5:Claude Codepull/4099/head
9 changed files with 432 additions and 42 deletions
@ -0,0 +1,72 @@
@@ -0,0 +1,72 @@
|
||||
// Copyright (c) 2026 Siegfried Pammer
|
||||
//
|
||||
// Permission is hereby granted, free of charge, to any person obtaining a copy of this
|
||||
// software and associated documentation files (the "Software"), to deal in the Software
|
||||
// without restriction, including without limitation the rights to use, copy, modify, merge,
|
||||
// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
|
||||
// to whom the Software is furnished to do so, subject to the following conditions:
|
||||
//
|
||||
// The above copyright notice and this permission notice shall be included in all copies or
|
||||
// substantial portions of the Software.
|
||||
//
|
||||
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
|
||||
// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
|
||||
// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
|
||||
// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
|
||||
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
|
||||
// DEALINGS IN THE SOFTWARE.
|
||||
|
||||
using System; |
||||
|
||||
using AwesomeAssertions; |
||||
|
||||
using ICSharpCode.ILSpyX.TreeView; |
||||
|
||||
using NUnit.Framework; |
||||
|
||||
namespace ICSharpCode.ILSpy.Tests.Controls; |
||||
|
||||
[TestFixture] |
||||
public class FlatListTreeNodeTests |
||||
{ |
||||
sealed class TestNode : SharpTreeNode |
||||
{ |
||||
readonly string text; |
||||
public TestNode(string text) => this.text = text; |
||||
public override object Text => text; |
||||
public override string ToString() => text; |
||||
} |
||||
|
||||
[Test] |
||||
public void GetNodeByVisibleIndex_WalkingPastTheEnd_ThrowsNamingIndexAndLength() |
||||
{ |
||||
var root = new TestNode("root"); |
||||
root.Children.Add(new TestNode("child")); |
||||
root.IsExpanded = true; |
||||
var listRoot = root.GetListRoot(); |
||||
listRoot.GetTotalListLength().Should().Be(2); |
||||
|
||||
// A restructure that happened under a reader leaves the augmented length disagreeing with
|
||||
// the structure it describes: the length says there is a node at this index, the descent
|
||||
// runs out of nodes before reaching it.
|
||||
listRoot.totalListLength = 5; |
||||
|
||||
var error = Assert.Throws<InvalidOperationException>( |
||||
() => SharpTreeNode.GetNodeByVisibleIndex(listRoot, 4)); |
||||
|
||||
error!.Message.Should().Contain("4").And.Contain("5"); |
||||
} |
||||
|
||||
[Test] |
||||
public void GetNodeByVisibleIndex_WithinTheList_ReturnsTheNodeAtThatIndex() |
||||
{ |
||||
var root = new TestNode("root"); |
||||
var child = new TestNode("child"); |
||||
root.Children.Add(child); |
||||
root.IsExpanded = true; |
||||
var listRoot = root.GetListRoot(); |
||||
|
||||
Assert.That(SharpTreeNode.GetNodeByVisibleIndex(listRoot, 0), Is.SameAs(root)); |
||||
Assert.That(SharpTreeNode.GetNodeByVisibleIndex(listRoot, 1), Is.SameAs(child)); |
||||
} |
||||
} |
||||
@ -0,0 +1,67 @@
@@ -0,0 +1,67 @@
|
||||
// Copyright (c) 2026 Siegfried Pammer
|
||||
//
|
||||
// Permission is hereby granted, free of charge, to any person obtaining a copy of this
|
||||
// software and associated documentation files (the "Software"), to deal in the Software
|
||||
// without restriction, including without limitation the rights to use, copy, modify, merge,
|
||||
// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
|
||||
// to whom the Software is furnished to do so, subject to the following conditions:
|
||||
//
|
||||
// The above copyright notice and this permission notice shall be included in all copies or
|
||||
// substantial portions of the Software.
|
||||
//
|
||||
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
|
||||
// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
|
||||
// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
|
||||
// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
|
||||
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
|
||||
// DEALINGS IN THE SOFTWARE.
|
||||
|
||||
using System; |
||||
using System.Linq; |
||||
|
||||
using ICSharpCode.ILSpyX.TreeView; |
||||
|
||||
using NUnit.Framework; |
||||
using NUnit.Framework.Interfaces; |
||||
|
||||
[assembly: TreeThreadAffinityGuard] |
||||
|
||||
// Deliberately outside any namespace: this is applied to the assembly, and the attribute has to be
|
||||
// nameable from the assembly-level attribute list.
|
||||
|
||||
/// <summary>
|
||||
/// Fails any test that mutated a displayed tree from a thread other than the tree's owner.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// The affinity check records into <see cref="TreeThreadAffinity.Violations"/> instead of relying
|
||||
/// on a throw, because tree mutation happens inside callers that catch Exception - the background
|
||||
/// decompile turns any exception a node raises into text in the output pane. A throw there is
|
||||
/// swallowed and the run still passes, so something has to read the collector, and it has to be
|
||||
/// per test: an assembly-level teardown failure is reported but does not fail the run or change
|
||||
/// the exit code. Debug builds only - the check compiles away in release, where the collector
|
||||
/// stays empty and this is a no-op.
|
||||
///
|
||||
/// A fixture that provokes violations on purpose clears the collector in its own
|
||||
/// <c>[TearDown]</c>, which runs before this.
|
||||
/// </remarks>
|
||||
[AttributeUsage(AttributeTargets.Assembly)] |
||||
public sealed class TreeThreadAffinityGuardAttribute : Attribute, ITestAction |
||||
{ |
||||
public ActionTargets Targets => ActionTargets.Test; |
||||
|
||||
public void BeforeTest(ITest test) |
||||
{ |
||||
TreeThreadAffinity.Clear(); |
||||
} |
||||
|
||||
public void AfterTest(ITest test) |
||||
{ |
||||
var violations = TreeThreadAffinity.Violations; |
||||
if (violations.Count == 0) |
||||
return; |
||||
TreeThreadAffinity.Clear(); |
||||
Assert.Fail($"{violations.Count} tree thread-affinity violation(s) were recorded while this test ran:" |
||||
+ Environment.NewLine |
||||
+ string.Join(Environment.NewLine, violations.Select(v => v.ToString()))); |
||||
} |
||||
} |
||||
Loading…
Reference in new issue