Hard crash due to bug in JavaParser

[xnhp]

To reproduce:

1. Run the DemoApp
2. In the menu, select "Language > Java"
3. As the last statement of a method body, enter Set<D and wait a second or so
4. The application crashes

Stacktrace:

ERROR: Token being duplicated: [TokenImpl: type=8388612; lexeme="}"; line=48; col=4; offs=1211; invalid=false]
java.lang.Exception: Stack trace
	at java.base/java.lang.Thread.dumpStack(Thread.java:1388)
	at org.fife.rsta.ac.java.rjc.lexer.Scanner.pushOntoStack(Scanner.java:105)
	at org.fife.rsta.ac.java.rjc.lexer.Scanner.yyPeek(Scanner.java:706)
	at org.fife.rsta.ac.java.rjc.parser.ASTFactory._getMemberDecl(ASTFactory.java:1084)
	at org.fife.rsta.ac.java.rjc.parser.ASTFactory._getClassBody(ASTFactory.java:405)
	at org.fife.rsta.ac.java.rjc.parser.ASTFactory._getNormalClassDeclaration(ASTFactory.java:1178)
	at org.fife.rsta.ac.java.rjc.parser.ASTFactory._getClassOrInterfaceDeclaration(ASTFactory.java:447)
	at org.fife.rsta.ac.java.rjc.parser.ASTFactory._getTypeDeclaration(ASTFactory.java:1451)
	at org.fife.rsta.ac.java.rjc.parser.ASTFactory.getCompilationUnit(ASTFactory.java:591)
	at org.fife.rsta.ac.java.JavaParser.parse(JavaParser.java:143)
	at org.fife.ui.rsyntaxtextarea.ParserManager.actionPerformed(ParserManager.java:163)
	at java.desktop/javax.swing.Timer.fireActionPerformed(Timer.java:317)
	at java.desktop/javax.swing.Timer$DoPostEvent.run(Timer.java:249)
	at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:313)
	at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:770)
	at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:721)
	at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:715)
	at java.base/java.security.AccessController.doPrivileged(Native Method)
	at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85)
	at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:740)
	at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124)
	at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109)
	at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
	at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90)

This is due to a call to System.exit in a sanity check done in pushOntoStack. JavaParser uses a stack for lookaheads etc. and one of the peek methods (yylex) apparently fails to properly pop a token in some cases, leaving the token on the stack. Then, when re-pushing it, they realise that the token is already on the stack, which shouldn’t happen.

[bobbylight]

I've put two changes into place for this:

• First, a fix for the actual issue. The problem was Scanner's currentResetTokenStack was getting polluted with duplicate tokens when it shouldn't, e.g. performing a yypeek() would result in duplicate values on this stack. This in turn caused all sorts of problems, including the panic described in this defect. I've put a fix for this in place.
• Next, I changed the System.exit() panic lines to throw new InternalError(). It's still pretty harsh, but hey, it's a sign there's a bug in the parser that needs to be addressed, and I didn't want to make too many changes to the actual scanner itself. This change at least provides a means for a consuming app to keep running rather than crashing completely, even if they must catch InternalErrors to avoid crashing the EDT.

I also updated the test Main class to verify both the JDK 1.8 source and another large project of mine. This isn't much of a test since all the code being parsed is valid, but at least it verifies that a large body of valid Java code is parsed properly with no errors. Actual unit tests of invalid/incomplete code would be amazing, but is a huge undertaking and is out of scope for this ticket unfortunately.

[xnhp]

Thanks for the quick fix!

I agree that throwing an exception there is more adequate.

Would you mind quickly outlining the reason why testing invalid code is hard to achieve here?

[bobbylight]

Maybe not hard, but it's a large amount of work to have all/most invalid code scenarios thoroughly covered by unit tests. I was pretty lazy and could have written a unit test for this case easily enough. I can put the test infrastructure in place but I probably won't get around to whipping this project into shape coverage-wise due to other projects and obligations. :(

All of the sister projects of RSTA are in the same boat - they were written without testing in mind and now the code bases would have to have tests retroactively written for them, which sucks. I've been working hard on RSTA the past year or so though, and it's sitting at 73% or so, which is pretty good for a Swing UI project (IMO). My primary testing focus with these projects has been getting RSTA up and over 80%.

[xnhp]

Of course, I can absolutely see that. I really do appreciate that you are still supporting these projects. Has been quite valuable to a lot of people so far.