Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/issue 3221 argument out of range exception #3271

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

1samrand
Copy link

issue :

Solution

  • Handle out of bound array index

@christophwille
Copy link
Member

This looks like fixing the symptom, but not the root cause - because some code must be picking an out-of-bounds index (maybe a race condition or some other problem). Preferably we should try to debug/find the actual culprit that hands over the out of bounds index.

@1samrand
Copy link
Author

Thank you for your feedback , I understand your concern about addressing the root cause rather than just the symptom. I'll take some time to investigate further to identify the underlying issue that might be causing the out-of-bounds index

@1samrand
Copy link
Author

1samrand commented Sep 8, 2024

Hi @christophwille,

After further investigation, it seems the issue is related to OnRender from WPF for a disconnected item. In this case, the Node is not null but has a count of 0.

However, as mentioned earlier, I'm still unsure why this is happening. If you have any insights, I would appreciate it if you could share with me.

The orange area highlights the specific case where this occurs (which I added to check):

image

@siegfriedpammer
Copy link
Member

diff --git a/ILSpy/Controls/TreeView/SharpTreeNodeView.cs b/ILSpy/Controls/TreeView/SharpTreeNodeView.cs
index a7cca8838..d144ca5c5 100644
--- a/ILSpy/Controls/TreeView/SharpTreeNodeView.cs
+++ b/ILSpy/Controls/TreeView/SharpTreeNodeView.cs
@@ -67,7 +67,6 @@ static SharpTreeNodeView()
                public override void OnApplyTemplate()
                {
                        base.OnApplyTemplate();
-                       LinesRenderer = Template.FindName("linesRenderer", this) as LinesRenderer;
                        UpdateTemplate();
                }

@@ -148,6 +147,7 @@ void OnIsEditingChanged()

                void UpdateTemplate()
                {
+                       LinesRenderer = Template.FindName("linesRenderer", this) as LinesRenderer;
                        var spacer = Template.FindName("spacer", this) as FrameworkElement;
                        spacer.Width = CalculateIndent();

@1samrand I found that this change fixes the issue and is likely the root cause. Can you confirm?

@1samrand
Copy link
Author

@christophwille I have tested by this change, but the ArgumentOutOfRangeException still occurs when accessing list[index].

@siegfriedpammer
Copy link
Member

How did you test it? I would like to take a look as well. I am following the steps given in the issue and I can no longer reproduce the error.

@1samrand
Copy link
Author

I have tested the scenario described here: #3221 (comment).
It seems that the issue persists, so perhaps there have been some changes in your branch that could be affecting the results.

If you're working on a specific branch, could you please share it with me so I can clone and test it as well?

@siegfriedpammer
Copy link
Member

Thanks for getting back to me. You are right, the patch I posted above does not fix the issue. Yeah, unfortunately there is no "nice" fix for the weird issues with LinesRenderer.

An idea for a simple non-invasive fix I had previously was:

image

We can either do your second suggestion (the additional sanity check in OnRender) or go with my fix... not sure which is better.

@1samrand
Copy link
Author

I have tested the changes you made, and I can confirm that the issue no longer occurs. I also prefer to use your solution moving forward

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants