MongoDBDialect uses shared Parboiled parser across multiple threads, while the parser is NOT thread safe

Description

When executing multiple (same) native queries against MongoDB (using createNativeQuery), following error happens easily:

java.lang.IllegalArgumentException: Cannot peek beyond the bottom of the stack
at org.parboiled.MatcherContext.runMatcher(MatcherContext.java:367) ~[parboiled-core-1.1.8.jar:1.1.8]
at org.parboiled.matchers.SequenceMatcher.match(SequenceMatcher.java:46) ~[parboiled-core-1.1.8.jar:1.1.8]
at org.parboiled.parserunners.BasicParseRunner.match(BasicParseRunner.java:77) ~[parboiled-core-1.1.8.jar:1.1.8]

Parboiled parsers are not thread safe - https://github.com/sirthias/parboiled/wiki/Thread-Safety

Yet the MongoDialect uses shared, static parser for native queries:

The access to this property is not synchronized.

Wrapping query execution (in the app) in synchronized (globalLock) mitigates this issue instantly and completely - but it hurts performance a lot as no queries can be executed in parallel.

Environment

Hibernate 5.3.7
OpenJDK 11
Spring Boot 2.1
MongoDB 3.4

Activity

Show:
gitdude
November 23, 2018, 3:09 AM

I would be willing to create pull request - but it would be good to know how do you want this to be implemented. IMHO parsing is reasonably fast to just do quick fix - put the parsing method parseNativeQuery into synchronized block would be an easy fix.
Other option would be to call newInstance every time the parser is used. Not sure which one is better, but given the usual simplicity of the native query, I would go with the synchronized block. Parsing the query is imho just very small fraction of query execution anyway.

Guillaume Smet
November 23, 2018, 4:39 AM

We usually try to avoid synchronized blocks.

The Parboiled doc says newInstance() is supposed to be fast once the parser has been created so I would use that.

gitdude
November 23, 2018, 9:20 AM
Fixed

Assignee

gitdude

Reporter

gitdude

Labels

None

Feedback Requested

None

Feedback Requested By

None

backPortable

None

Suitable for new contributors

None

backportDecision

None

backportReEvaluate

None

Components

Fix versions

Affects versions

Priority

Blocker