Possible Race Condition in ToDateTokenizer.initCache()/.initTokens() #3687
Description
I recently ran across the below exception (edited to remove customer info). It occurs when the code attempts to load data concurrently from multiple sources. The list being iterated is the result of a call to ToDateTokenizer.FormatTokenEnum.getTokensInQuestion(p.getFormatStr()) Reading the code for ToDateTokenizer I see that the initCache() method is synchronized, but it is modifying the statically held CACHE entries. My best hypothesis at this point for how we hit the below exception is that the code in the stacktrace was iterating an entry from the cache that was incompletely initialized by another thread. This would (I think) be possible because the first check for size() <= 0 can potentially pass while the first thread is still adding values during initCache(). In such a case the second thread no-longer enters the monitor and therefore has no "happens-before" relationship or guarantee that initialization has completed, only that a single cache entry was observable.
Naturally, since the project was using 1.4.197, I asked if the current h2 versions had this problem as well, and reading the current code on master, it's changed completely. However, there is still a statically held TOKENS which is initialized without synchronization, and held in a non-volatile static field. I think the current code has solved the problem that causes the CME I experienced by converting the map to an array of lists, which is only exposed after iteration is complete, but now has a race wherein the cache may be replaced by any thread that reads the TOKENS field as null and it is very difficult to reason about the safety of the results since there are no happens-before edges (unless I missed one) in the new code path, so I'm not even sure we couldn't still get a CME if an iterating thread paused, and previously unobservable elements in the list became observable before it resumed.
I think for statically held resources that are initialized lazily there should be some form of synchronization or use of volatile. One quick fix might be to use an AtomicReferenceArray but the performance implications of that obviously need to be tested carefully. Also I note that the List[] is always of size 25 (why no Z?) so I don't se any reason it (or an AtomicRerferenceArray) couldn't be final and initialized empty early on. (one empty 25 element array per classloader probably isn't too heavy since any code that references this class will likely initialize it anyway).
org.skife.jdbi.v2.exceptions.UnableToCreateStatementException: org.h2.jdbc.JdbcSQLException: General error: "java.util.ConcurrentModificationException" [50000-197] [statement:”REDACTED”, arguments:{ positional:{}, named:{null:0,maxHashBuckets:6,hashBucket:2,rowNumber:0}, finder:[]}]
at org.skife.jdbi.v2.SQLStatement.internalExecute(SQLStatement.java:1307)
at org.skife.jdbi.v2.Query.iterator(Query.java:240)
at com.customer.search.model.documents.impl.DbShardedQuerySpliterator.makeRetrievalIterator(DbShardedQuerySpliterator.java:198)
at com.customer.search.model.documents.impl.DbShardedQuerySpliterator.lambda$startRetrievalAtPage$5(DbShardedQuerySpliterator.java:219)
at com.customer.search.RetriableTask.lambda$tryWithSleepUnlessPredicateTR$12(RetriableTask.java:171)
at com.customer.search.RetriableTask.tryWithSleepUnlessPredicate(RetriableTask.java:133)
at com.customer.search.RetriableTask.tryWithSleepUnlessPredicate(RetriableTask.java:111)
at com.customer.search.RetriableTask.tryWithSleepUnlessPredicateTR(RetriableTask.java:170)
at com.customer.search.RetriableTask.doExponentialBackoff(RetriableTask.java:106)
at com.customer.search.model.documents.impl.DbShardedQuerySpliterator.startRetrievalAtPage(DbShardedQuerySpliterator.java:257)
at com.customer.search.model.documents.impl.DbShardedQuerySpliterator.startRetriever(DbShardedQuerySpliterator.java:155)
at com.customer.search.model.documents.impl.DbShardedQuerySpliterator.lambda$startRetrievers$1(DbShardedQuerySpliterator.java:138)
at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:834)
Caused by: org.h2.jdbc.JdbcSQLException: General error: "java.util.ConcurrentModificationException" [50000-197]
at org.h2.message.DbException.getJdbcSQLException(DbException.java:357)
at org.h2.message.DbException.get(DbException.java:168)
at org.h2.message.DbException.convert(DbException.java:307)
at org.h2.message.DbException.toSQLException(DbException.java:280)
at org.h2.message.TraceObject.logAndConvert(TraceObject.java:357)
at org.h2.jdbc.JdbcConnection.prepareStatement(JdbcConnection.java:308)
at com.zaxxer.hikari.pool.ProxyConnection.prepareStatement(ProxyConnection.java:337)
at com.zaxxer.hikari.pool.HikariProxyConnection.prepareStatement(HikariProxyConnection.java)
at org.skife.jdbi.v2.DefaultStatementBuilder.create(DefaultStatementBuilder.java:54)
at org.skife.jdbi.v2.SQLStatement.internalExecute(SQLStatement.java:1303)
... 16 common frames omitted
Caused by: java.util.ConcurrentModificationException: null
at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1043)
at java.base/java.util.ArrayList$Itr.next(ArrayList.java:997)
at org.h2.util.ToDateParser.parse(ToDateParser.java:273)
at org.h2.util.ToDateParser.getTimestampParser(ToDateParser.java:76)
at org.h2.util.ToDateParser.toDate(ToDateParser.java:350)
at org.h2.expression.Function.getValueWithArgs(Function.java:1407)
at org.h2.expression.Function.getValue(Function.java:567)
at org.h2.expression.Function.optimize(Function.java:2326)
at org.h2.expression.Function.optimize(Function.java:2131)
at org.h2.expression.Comparison.optimize(Comparison.java:177)
at org.h2.expression.ConditionAndOr.optimize(ConditionAndOr.java:131)
at org.h2.expression.ConditionAndOr.optimize(ConditionAndOr.java:130)
at org.h2.command.dml.Select.prepare(Select.java:861)
at org.h2.command.Parser.prepareCommand(Parser.java:283)
at org.h2.engine.Session.prepareLocal(Session.java:611)
at org.h2.engine.Session.prepareCommand(Session.java:549)
at org.h2.jdbc.JdbcConnection.prepareCommand(JdbcConnection.java:1247)
at org.h2.jdbc.JdbcPreparedStatement.<init>(JdbcPreparedStatement.java:76)
at org.h2.jdbc.JdbcConnection.prepareStatement(JdbcConnection.java:304)
... 20 common frames omitted