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

Silent crash on Windows with very long line #331

Open
Gama11 opened this issue Feb 13, 2017 · 9 comments
Open

Silent crash on Windows with very long line #331

Gama11 opened this issue Feb 13, 2017 · 9 comments
Labels

Comments

@Gama11
Copy link
Member

Gama11 commented Feb 13, 2017

Checkstyle dies on this line:

https://github.com/HaxeFlixel/flixel/blob/4.2.0/flixel/graphics/frames/FlxBitmapFont.hx#L31

And apparently only on Windows.

Easiest way to reproduce is to clone the flixel repo and run haxelib run checkstyle -s . -progress - FlxBitmapFont.hx will be the last file displayed before it exits.

I know that line is the one causing the problem because it works fine after it's removed.

@AlexHaxe
Copy link
Member

I took a quick look at the issue on a Windows 10 machine, here is what I found:
neko.exe crashes in regexp.ndll with a stackoverflow
I have narrowed it down to line based checks (tested with IndentationCharacter, TabForAligning, TrailingWhitespace), so I guess it has to do with the multiline / string detection stuff.

@Gama11
Copy link
Member Author

Gama11 commented Feb 13, 2017

Hm, I guess it must be a Neko bug then. :/ Perhaps it's trying to run a regex over that long line and the regex engine can't handle strings that long on Windows for some reason?

@AlexHaxe
Copy link
Member

I think it might be more of an "evil" regex problem ( https://www.owasp.org/index.php/Regular_expression_Denial_of_Service_-_ReDoS )
Maybe it works on Linux / OSX, because neko uses larger stacksizes on those systems.
Just wild guesses, I haven't looked any further.

A solution might be to change our regexes, so that there is less need for backtracking or recursion. Though I'm not sure if that's possible.

@Gama11
Copy link
Member Author

Gama11 commented Feb 13, 2017

We should definitely at least find a workaround for crashes like this... Though that might be tricky. Maybe some heuristic like "if the line is longer than x characters, don't risk it". :/

@AlexHaxe
Copy link
Member

As a workaround we could make lines above 1000 (or 2000, needs testing) spit out a checkstyle message for all line based checks. So that users get some kind of feedback on those files, and can fix line length or exclude checks.

A more stable solution might be to go hybrid as proposed in #278, where comment and string detection comes though token tree. But that will come with some performance impact.

@Gama11
Copy link
Member Author

Gama11 commented Feb 13, 2017

I doubt that's necessary, this seems like a pretty rare edge case considering it hasn't come up until now.

@cmandlbaur
Copy link

Just ran into what seems like this issue as well. In particular on the TabForAligning, EmptyLines, and IndentationCharacter checks.

We don't use the TrailingWhitespace check. All these checks extend LineCheckBase.

Neko returning 3221225725, which is apparently a stack overflow?

Happy to investigate a fix and do a pull request. What version of haxe should I be using to build?

@cmandlbaur
Copy link

To add it was also due to an extremely long line in a test.

I understand the priority is pretty low due to it being an edge case, but its taken me a good couple hours to track down. So again, let me know what compiler version I should be using and I will attempt a fix.

@AlexHaxe
Copy link
Member

I would recommend Haxe 4rc5 for compiling git checkstyle.

I haven't done extensive tests, but it seems running a nodejs version of checkstyle might be bit more reliable.

Maybe we need to do the same nodejs switcheroo as we do with formatter. And maybe I should make that checkstyle release, that I keep postponing...

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

No branches or pull requests

3 participants