-
Notifications
You must be signed in to change notification settings - Fork 114
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
100% Freeze on any catastrophic backtracking #104
Comments
We call into oniguruma, which is written in C++. Control never returns and it does not have a timeout option of any sorts. In the past, I have "debugged" such cases only by logging in Here I would add a printf for |
@alexandrudima Is the call into oniguruma from this upstream https://github.com/kkos/oniguruma? I can post an issue on there. Funny enough, I'd bet this issue is much easier to fix in C++ thanks to multithreading. Thanks for the debugging advice too. |
We have plans to improve things for VS Code. See microsoft/vscode#77140 where we want to look into running the TM grammars on separate threads. We consume |
That is an awesome post, thanks for linking me to it. |
Reasons a change is needed:
Recommended change:
Find some way to have a timeout on a per-line check. If the line goes past the timeout (ex: 5 seconds) the parser just groups the entire rest of the document (all of the remaining lines) as one scope, and closes any unclosed scopes. The reason being: that it would fail safely (no freeze) but would fail badly (not-user friendly) so that the failsafe is not used a a crutch to allow poorly written regex to become acceptable. Failing like this would also give the dev the ability to see exactly which line the error occurred at.
Difficulty:
Probably pretty difficult. The only solutions I can imagine are that the code needs to be async (I'd assume its currently synchronous), or some advanced tool like worker threads would need to be used (pass the current line to the worker, let it start working, if the worker doesn't finish in the timeout, then fail)
The text was updated successfully, but these errors were encountered: