DateRecognizerFilterFactory shares one DateFormat across threads, so concurrent analysis throws NumberFormatException (since 5.5.0)

DateRecognizerFilterFactory (SPI name dateRecognizer) builds one java.text.DateFormat in its constructor (DateRecognizerFilterFactory.java:53, :60) and create() passes that same object to every DateRecognizerFilter it creates (:72-74). An Analyzer keeps one set of components per thread (Analyzer.java:91-96), so CustomAnalyzer calls create() once per thread on the same factory (CustomAnalyzer.java:161-168) and every thread's filter parses with the one shared format. The JDK documents DateFormat as not synchronized. When two threads parse at once, DateFormat.parse throws NumberFormatException (multiple points, For input string: "") or ArrayIndexOutOfBoundsException (and, in one launch each on 9.0.0 and 9.12.3, a single ClassCastException), and DateRecognizerFilter.accept() catches only ParseException (DateRecognizerFilter.java:50-61), so the exception escapes incrementToken(). With an IndexWriter, whose indexing threads share one analyzer, that addDocument throws and the document is not indexed; in every run with assertions off the writer stayed open. No special input or scale is needed, and it fails with assertions on and off. It fails on all 10 releases run, from 5.5.0 to 10.5.2, and on main; the code is unchanged since 5.5.0. Passing (DateFormat) dateFormat.clone() in create() fixes it; with that change all of :lucene:analysis:common:test passes (1,911 tests, 18 skipped, 0 failures).

• DateRecognizerFilter: a token filter in lucene/analysis/common that keeps a token only if a java.text.DateFormat can parse it ("Filters all tokens that cannot be parsed to a date"). DateRecognizerFilterFactory builds it by name (dateRecognizer) for CustomAnalyzer or a Solr-style schema, with optional datePattern and locale arguments.
• Per-thread components: an Analyzer builds its tokenizer and filters once per thread and reuses them (Analyzer.java:91-96, a CloseableThreadLocal). CustomAnalyzer builds them by calling create() on the same factory objects in every thread (CustomAnalyzer.java:161-168).
• DateFormat: the JDK's date parser. Its class javadoc says: "Date formats are not synchronized. It is recommended to create separate format instances for each thread."

1. The factory builds one format in its constructor: private final DateFormat dateFormat; (DateRecognizerFilterFactory.java:53), this.dateFormat = getDataFormat(get(args, DATE_PATTERN)); (:60). getDataFormat (:84-90) returns a SimpleDateFormat for datePattern, else DateFormat.getDateInstance(DateFormat.DEFAULT, locale).
2. create() passes that same object to every filter: return new DateRecognizerFilter(input, dateFormat); (:72-74). The filter keeps the reference (DateRecognizerFilter.java:42-48).
3. So with an analyzer used by 4 threads, 4 filters share one DateFormat. The state the race damages is mainly a Calendar and a NumberFormat, the two fields DateFormat.clone() deep-copies (JDK source; SimpleDateFormat.parse also writes a few minus-sign fields in a one-time setup). The JDK's DecimalFormat collects the digits of each number field in one buffer and hands them to Long.parseLong or Double.parseDouble, which is consistent with the garbled strings seen, such as 202420024024E202420024024E4.
4. accept() (DateRecognizerFilter.java:50-61) calls dateFormat.parse(...) and catches only ParseException, so the NumberFormatException or ArrayIndexOutOfBoundsException escapes incrementToken().
5. At index time an exception from the token stream is non-aborting: the document is marked deleted (IndexingChain.java:1989-1994, DocumentsWriterPerThread.java:302-307), addDocument throws, and the writer stays open. At query time, building the query fails.

Single-threaded, the factory-built and the constructor-built analyzers give identical tokens. The exceptions are mostly NumberFormatException, then ArrayIndexOutOfBoundsException; the version sweep also saw one ClassCastException in one launch each on 9.0.0 and 9.12.3. With JDK system assertions on (-esa), a JDK-internal calendar AssertionError can fire instead and leave the shared calendar broken; that is a test-JVM artifact (-ea -dsa gives the same rates as -da).

• Who is exposed. Users of the dateRecognizer factory: through CustomAnalyzer, the SPI name, or a Solr-style schema such as the factory javadoc's own example. No Lucene analyzer adds it by default, and a read-only code search of Solr, Elasticsearch and OpenSearch found no default configuration that uses it.
• What it takes. More than one thread using the analyzer: IndexWriter is "completely thread safe" and its indexing threads share the one analyzer of its config (IndexWriter.java:167-168); concurrent query analysis does the same. No special input: with datePattern, any date-shaped token can race; with no datePattern, the default format needs a tokenizer that keeps spaces (keyword): a whitespace token has no ordinary space, so it does not match Jan 15, 2024 (a date written with non-breaking spaces stays one token and does parse on JDK 25).
• What the damage looks like. An unchecked exception reported to the caller: the document is rejected (marked deleted, other documents indexed, the writer stays open), or the query cannot be built. The caller can retry. No silent wrong tokens were seen. These are observations on JDK 25 with the tested configurations; another JDK could fail differently, for example with an Error, on which IndexWriter closes itself (IndexWriter.java:162-165).
• Not affected. Single-threaded use, and the filter built through its public constructor with one DateFormat per component.
• Scale and rate. No data volume needed. Per operation it is rare: in the recorded runs that failed, from about 15 per million (3 of 200,000 addDocument calls) to 0.53% (1,065 of 200,000 analyses), depending on load and configuration, and some runs saw none. Even so, a 4-thread test fails in every launch.
• How likely. Plausible (registry): ordinary multi-threaded use of an opt-in filter; no report in ten years.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: expected no exception, but analysis threw java.lang.NumberFormatException: multiple points
       org.apache.lucene.analysis.miscellaneous.TestDateRecognizerFactoryThreads.testTwoThreadsShareOneAnalyzer(TestDateRecognizerFactoryThreads.java:56)
     Caused by: java.lang.NumberFormatException: multiple points
       java.base/jdk.internal.math.FloatingDecimal.checkMultiplePoints(FloatingDecimal.java:2308)
       java.base/jdk.internal.math.FloatingDecimal.readJavaFormatString(FloatingDecimal.java:1909)
       java.base/jdk.internal.math.FloatingDecimal.parseDouble(FloatingDecimal.java:110)
       java.base/java.lang.Double.parseDouble(Double.java:971)
       java.base/java.text.DigitList.getDouble(DigitList.java:173)
       [...]

• create() output runs on many threads by design. Analyzer.java:93-96 ("reusing the same set of components per-thread"), CustomAnalyzer.java:161-168 (one create() per thread on shared factories) and IndexWriter.java:167-168 ("completely thread safe, meaning multiple threads can call any of its methods, concurrently").
• The JDK contract. "Date formats are not synchronized. It is recommended to create separate format instances for each thread. If multiple threads access a format concurrently, it must be synchronized externally." The factory creates the format privately and never exposes it.
• Lucene's tests expect it. BaseTokenStreamTestCase.checkRandomData repeats the same analysis in 2-4 threads: "we do the EXACT same thing we did before in each thread, so this should only really fail from another thread if it's an actual thread problem" (BaseTokenStreamTestCase.java:1086-1089). The nightly TestFactories runs it on every SPI factory it can build without arguments, dateRecognizer included, but behind a whitespace tokenizer, whose tokens practically never parse with the default format.
• Sibling code. PhoneticFilterFactory.getEncoder(): "Must be thread-safe." "Commons-Codec doesn't offer any thread-safe guarantees so we must play it safe and instantiate every time." (PhoneticFilterFactory.java:160-164, called from create()). DateTools keeps per-thread clones of its formats (DateTools.java:55-61; :192 "should be cloned before use, since it's not threadsafe").
• Precedent. LUCENE-940 (#2015, 2007, "SimpleDateFormat used in a non thread safe manner") and LUCENE-6564 (#7622, 2015, the static shared SimpleDateFormat of PrintStreamInfoStream) fixed a shared SimpleDateFormat as a bug in other components. LUCENE-1472 (#2546) later replaced DateTools' synchronized shared formats with per-thread copies, for performance: those formats were already guarded by a lock.
• No decision says otherwise. No javadoc restricts the factory to one thread. The introducing issue (LUCENE-6973, #8030, 40 comments) discusses locales and tests, never threads.

The strongest case for "intended": (a) no javadoc on TokenFilterFactory or this factory promises that create() output is thread-safe, and Analyzer never says "thread-safe" in so many words; (b) the filter's two-argument constructor takes a caller-owned DateFormat, and the JDK says a shared one "must be synchronized externally", so sharing could be called the caller's problem; (c) the default configuration with a whitespace tokenizer practically never parses, so the affected configurations are narrow; (d) nobody reported it in ten years.

Why it fails: (a) the per-thread component design, IndexWriter's shared analyzer, the multi-threaded checkRandomData run on every factory it can build (this one included) and the PhoneticFilterFactory comment all strongly imply that create() output must survive concurrent use (no single javadoc sentence states it); (b) here Lucene's own factory creates the format and hides it, so the user cannot synchronize it; (c) the javadoc's own configuration uses a datePattern, which is enough; (d) the upstream tests never parse dates concurrently and the 2016 review never considered threads. The cross-model reviews, the devil's advocate and the adjudication all rated the intended-behaviour case weak or none.

Two options were applied in a scratch worktree of a9779ef3c193 and run through gradle: A with the new test, the module's tests, the nightly TestFactories and check; B with the DateRecognizer tests. The worktree was removed afterwards.

One line in DateRecognizerFilterFactory.create() (DateRecognizerFilterFactory.java:72-74). The factory keeps its instance as a template; every filter gets a deep copy.

   @Override
   public TokenStream create(TokenStream input) {
-    return new DateRecognizerFilter(input, dateFormat);
+    // DateFormat is not thread-safe: give every filter (one per analysis thread) its own copy
+    return new DateRecognizerFilter(input, (DateFormat) dateFormat.clone());
   }

Trade-offs

• One clone per create() call, not per token or document. With CustomAnalyzer (Analyzer's default GLOBAL_REUSE_STRATEGY) that is once per thread and analyzer; PER_FIELD_REUSE_STRATEGY builds one chain per thread and field, and direct callers of the factory get one copy per call.
• DateFormat.clone() deep-copies the calendar and the number format, and SimpleDateFormat.clone() the date format symbols, so the copies do not share the calendar and number-format state that parsing changes (the compiled pattern and the century-start date stay shared; parsing does not change them). This holds for the formats the stock getDataFormat builds; an override that returns a custom DateFormat needs a clone() that copies its mutable state.
• An invalid datePattern still fails when the factory is built.

What the checks validated

• TestDateRecognizerFactoryThreads passes with assertions off and on, and all 10 repetitions with -Ptests.iters=10; with TestDateRecognizerFilter and TestDateRecognizerFilterFactory: 5 tests, 0 failures (stock: 1 failure).
• All of :lucene:analysis:common:test with assertions on: 340 classes, 1,911 tests, 18 skipped, 0 failures, 0 errors.
• TestFactories with -Ptests.nightly=true passes.
• :lucene:analysis:common:check -x test passes (checkGoogleJavaFormat, ecjLint, forbiddenApis), with the new test file.
• Earlier: the finder's test, the independent reproducer's 5 tests (numDocs 40,000 of 40,000) and an IndexWriter test pass with the same change; the adjudication's re-run passed 5 of 5.

Keep the pattern instead of the format, call getDataFormat(datePattern) in create(), and call it once in the constructor too so an invalid pattern still fails early.

Trade-offs

With the stock getDataFormat the same effect as A, with a few more changed lines and a little more work per create() (the pattern is parsed again). B calls a subclass's override of getDataFormat on every create(), while A clones the result the constructor got from it; an override that returned the same instance every time would bring the race back under B, not under A.

What the checks validated

• TestDateRecognizerFactoryThreads, TestDateRecognizerFilter and TestDateRecognizerFilterFactory pass with assertions off and on (5 tests, 0 failures). The module suite, check and repetitions were run only for A.

this.dateFormat = (DateFormat) dateFormat.clone() in DateRecognizerFilter.java:42-48.

Trade-offs

Also protects callers of the public two-argument constructor who share one instance, but changes that constructor's semantics (the filter no longer uses the caller's object). A design decision; not tried.

Lock the shared format around parse in accept().

Trade-offs

Serializes every analysis thread on one lock; Lucene replaced exactly that in DateTools with per-thread copies (LUCENE-1472). A ThreadLocal would work, but a copy per filter already is one per thread.

Treat any exception from parse as "not a date".

Trade-offs

Turns the race into silent dropping of valid dates, which is worse than an exception.

The JDK's immutable, thread-safe alternative.

Trade-offs

An API change (the public constructor takes a DateFormat) and different, stricter default parsing; not a bug fix.

Method: a concurrency stress differential (registry tier semi-formal: a hypothesis from a static inventory, witnessed by a differential, not by a formal tool).

• The hunt unit compared every SPI analysis factory with the public constructor it wraps. A static inventory extracted the constructor and create() bodies of all 167 SPI factory registrations; reading the factory fields showed that DateRecognizerFilterFactory passes one factory-owned DateFormat to every filter.
• A concurrency differential then ran every constructible factory configuration (210) on the stock main jars: the tokens of a single-threaded run against 3-4 concurrent threads on the same analyzer locally, and 8 on separate build machines. 209 configurations were clean; this was the only mismatch.
• The finder reduced it to the public-API JUnit with a per-component control, measured the hit rate (50 of 50 JVM launches with assertions on and off) and the exception kinds, and checked the one-line clone fix.