Skip to content

Loops / infinite recursion #6

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

Closed
infostreams opened this issue Nov 7, 2021 · 9 comments
Closed

Loops / infinite recursion #6

infostreams opened this issue Nov 7, 2021 · 9 comments

Comments

@infostreams
Copy link

If you try to diff structures that have a loop in it, your diff algorithm will hang and enter infinite recursion and therefore exhaust system resources until the tab or the process hangs.

@AsyncBanana
Copy link
Owner

I am looking for a solution that will not decrease performance or increase size much, but it might take a bit to figure something out. If you have any good ideas on how to do this, feel free to submit a PR.

@infostreams
Copy link
Author

@AsyncBanana
Copy link
Owner

Thanks! However, the problem is those algorithms take up more space, and, more importantly, they would significantly decrease performance for objects. Currently, this probably will not be solved, although I might eventually make an extension to the core library for checking for circular references.

@infostreams
Copy link
Author

Ha! That's a bad decision. Who cares about speed if using your library could potentially hang MY application? This will bite some people, I can guarantee it.

@AsyncBanana
Copy link
Owner

Generally, you should avoid practices that could cause circular references as it is. However, if you cannot do that, you can implement circular reference checking before diffing the object.

@infostreams
Copy link
Author

Ok, whatever 🤷‍♂️

@AsyncBanana
Copy link
Owner

@infostreams First of all, sorry for not finding a solution earlier.
I have done a bit more research, and I think I have found a way to deal with this without causing a large size and performance cost by taking advantage of the preexisting loop. I should be able to release a bug fix that fixes this problem soon.

@AsyncBanana AsyncBanana reopened this Nov 7, 2021
@infostreams
Copy link
Author

Awesome! Sorry for having been an ass about this, I could have done that better. Thanks for taking it seriously!

@AsyncBanana
Copy link
Owner

It is not your fault, I was making this lower priority than it should have been, and I did not think of a solution that I should have thought of before.
I have released a new version, 1.1.2, that should have this fixed. If you have any more problems, let me know.

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