StringLiteral.getLiteralValue is not thread-safe

[martinlippert]

It looks like StringLiteral.getLiteralValue is throwing IllegalStateExceptions in some cases when used from multiple threads, most likely due to the internal scanner object being used in a stateful way.

[jukzi]

Please paste the Exception Stacktrace and why it is used in multiple threads

[martinlippert]

Example stack traces can be found here: spring-projects/sts4#1434 - but this comes from the Spring Tools using JDT Core to parse and analyze source code, so the stack traces have no origins in JDT or Eclipse itself.

We use the AST over there to implement content-assist, do additional validations, and extract index information from a Spring perspective.

[jukzi]

That reference only shows a IllegalArgumentException, not a IllegalStateException , also the line number does not match master.
The IllegalArgumentException complains tokenType!=TerminalTokens.TokenNameStringLiteral

I don't see a connection to concurrent use. You should be able to set a breakpoint to find the sourcecode in question that cannot be parsed.

[martinlippert]

That reference only shows a IllegalArgumentException, not a IllegalStateException , also the line number does not match master. The IllegalArgumentException complains tokenType!=TerminalTokens.TokenNameStringLiteral

You are right, my bad, it is an IllegalArgumentException, not an IllegalStateException. The line numbers are from using JDT Core 3.39.

I don't see a connection to concurrent use. You should be able to set a breakpoint to find the sourcecode in question that cannot be parsed.

I am not yet convinced about this not being related to concurrent use, but let me try to dig deeper here. Always hard to reproduce those things, but let me see what I can find out with our hints here. Thanks so much for looking into this so quickly and for providing the feedback here.

[jukzi]

This issue still has not enough information to process.

[martinlippert]

@jukzi I am on it and will provide more details / information as soon as possible... :-)

[martinlippert]

I created a code snippet to reproduce this issue with concurrent calls to getLiteralValue:

ASTParser parser = ASTParser.newParser(AST.JLS21);
parser.setSource("\"hello\\nworld\"".toCharArray());
parser.setKind(ASTParser.K_EXPRESSION);
ASTNode ast = parser.createAST(null);

final StringLiteral literal = (StringLiteral) ast;

ExecutorService executorService = Executors.newFixedThreadPool(10);
List<CompletableFuture<Void>> futures = new ArrayList<>();
for (int i = 0; i < 100; i++) {
	CompletableFuture<Void> future = CompletableFuture.runAsync(() -> {
		try {
			literal.getLiteralValue();
		} catch (Exception e) {
			e.printStackTrace();
		}
	}, executorService);
	futures.add(future);
}
CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])).join();
executorService.shutdown();

This should result in a few exceptions being thrown due to issues within getLiteralValue due to concurrent access to it.

Looking at the implementation of getLiteralValue it seems that the scanner object is the culprit. It is used to scan the escaped value in a stateful way, but calls to getLiteralValue always use the same scanner object.

[jukzi]

was already reported in https://bugs.eclipse.org/bugs/show_bug.cgi?id=508586

[jukzi]

as per https://bugs.eclipse.org/bugs/show_bug.cgi?id=508586#c14 the scanner should be cloned. @martinlippert can you contribute a fix?