Shawn Smith created JENA-2356:
---------------------------------
Summary: Race condition with QueryEngineRegistry and
UpdateEngineRegistry init()
Key: JENA-2356
URL: https://issues.apache.org/jira/browse/JENA-2356
Project: Apache Jena
Issue Type: Bug
Components: ARQ
Affects Versions: Jena 4.9.0
Environment: Java 17 on Linux.
Reporter: Shawn Smith
I encountered this exception in an application where two threads concurrently
tried to execute a SPARQL query for the first time since the application
started:
{code:java}
java.util.ConcurrentModificationException: null
at
java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1013)
at java.base/java.util.ArrayList$Itr.next(ArrayList.java:967)
at
org.apache.jena.sparql.engine.QueryEngineRegistry.find(QueryEngineRegistry.java:94)
at
org.apache.jena.query.QueryExecutionBuilder.build(QueryExecutionBuilder.java:98)
at
org.apache.jena.query.QueryExecutionFactory.make(QueryExecutionFactory.java:622)
at
org.apache.jena.query.QueryExecutionFactory.make(QueryExecutionFactory.java:602)
at
org.apache.jena.query.QueryExecutionFactory.create(QueryExecutionFactory.java:146)
at
org.apache.jena.query.QueryExecutionFactory.create(QueryExecutionFactory.java:158)
... {code}
The application doesn't use {{QueryEngineRegistry}} directly in any way, and
the relevant code hasn't changed recently. I clearly just got unlucky due to a
race condition in how {{QueryEngineRegistry}} initializes itself.
At first glance, {{QueryEngineRegistry}} initialization looks fine due to the
use of a static initializer:
{code:java}
public class QueryEngineRegistry
{
...
static { init(); }
...
static QueryEngineRegistry registry = null;{code}
But it turns out that the relative location of the {{init()}} and the field
declaration matters. Once compiled, the code ^^ is equivalent to this:
{code:java}
public class QueryEngineRegistry
{
...
static { init(); }
...
static QueryEngineRegistry registry;
static { registry = null; } <== UNDOES THE EFFECT OF THE static init(){code}
So the static initializer call to {{init()}} ends up having no effect because
the field initialization happens afterwards, nulling out the registry created
by {{{}init(){}}}.
Instead, initialization falls to the code in the {{get()}} method:
{code:java}
static public QueryEngineRegistry get()
{
if ( registry == null )
init();
return registry;
}
...
private static synchronized void init()
{
registry = new QueryEngineRegistry();
registry.add(QueryEngineMain.getFactory());
registry.add(QueryEngineFactoryWrapper.get());
}{code}
Despite using {{synchronized}} in {{{}init(){}}}, this initialization strategy
is not thread-safe. Once the first line of {{init()}} has executed, a
concurrent thread that calls {{get()}} may receive a partially-initialized
instance of {{{}QueryEngineRegistry{}}}. I believe that is what happened to me
to trigger the stack trace above.
Fixing the race condition can be as simple as the following change, which fixes
the issue with the static initializer:
{code:java}
< static QueryEngineRegistry registry = null;
> static QueryEngineRegistry registry; {code}
But it seems like a good idea to cleanup the broken initialization code in
get(), probably by simply removing it and relying on the static initializer.
And note {{UpdateEngineRegistry}} uses the same initialization pattern.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
