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

Reverse interval not validated, then causes "Excessive recursion detected" exception #3

Closed
felix-b opened this issue May 4, 2024 · 3 comments

Comments

@felix-b
Copy link

felix-b commented May 4, 2024

Hi @jamarino!
Thanks again for this library, so far it does exactly what I need.
There was a minor issue I came across.
I happened to incorrectly add a reversed interval to QuickIntervalTree (from greater than to).
No validation errors occurred.
However, when attempted to run Query, I got a System.Exception saying "Excessive recursion detected".
If reverse intervals cause such behavior, wouldn't it be better to validate from and to in the Add method?
Could save the users some time looking for their bugs.

Here is the reproduction:

var tree = new QuickIntervalTree<int, string>();

tree.Add(100, 200, "abc");
tree.Add(10, 20, "def");
tree.Add(-100, -200, "uvw"); // incorrect; should be (-200, -100)
tree.Add(50, 150, "xyz");

tree.Query(-150, 150).ToArray(); // System.Exception here
System.Exception : Excessive recursion detected, aborting to prevent stack overflow. Please check thread safety.
   at Jamarino.IntervalTree.QuickIntervalTree`2.<Build>g__BuildRec|18_0(Int32 min, Int32 max, Int32 nodeIndex, Int32 recursionLevel)
   at Jamarino.IntervalTree.QuickIntervalTree`2.<Build>g__BuildRec|18_0(Int32 min, Int32 max, Int32 nodeIndex, Int32 recursionLevel)
   at Jamarino.IntervalTree.QuickIntervalTree`2.<Build>g__BuildRec|18_0(Int32 min, Int32 max, Int32 nodeIndex, Int32 recursionLevel)
.......
@felix-b
Copy link
Author

felix-b commented May 4, 2024

Here is a pull request -- feel free to take a look:
#4

@jamarino
Copy link
Owner

jamarino commented May 5, 2024

Hello again, @felix-b !
What a silly oversight on my part! I'm glad the recursion check paid off... 😅

Your suggested changes look good to me and align well with the behaviour of RangeTree (for compatibility). I'll get 'em merged and published. Thank you!

@jamarino
Copy link
Owner

jamarino commented May 5, 2024

Merged, tagged, pushed and published ✔️ Just waiting for nuget.org to validate the upload, it should be available as version 1.2.1 momentarily.

Thanks again! Let me know if you encounter any other oddities or missing features 😃

@jamarino jamarino closed this as completed May 5, 2024
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

No branches or pull requests

2 participants