You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@jena.apache.org by "Andy Seaborne (Jira)" <ji...@apache.org> on 2023/10/21 13:08:00 UTC

[jira] [Updated] (JENA-2356) Race condition with QueryEngineRegistry and UpdateEngineRegistry init()

     [ https://issues.apache.org/jira/browse/JENA-2356?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Andy Seaborne updated JENA-2356:
--------------------------------
    Fix Version/s: Jena 4.10.0

> 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
>            Assignee: Andy Seaborne
>            Priority: Minor
>             Fix For: Jena 4.10.0
>
>
> 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: jira-unsubscribe@jena.apache.org
For additional commands, e-mail: jira-help@jena.apache.org