FSTCompiler.Builder: a second build() shares the first compiler's default output, so the second FST misses its own keys

FSTCompiler.Builder.build() creates the default on-heap output when the caller set no dataOutput, but it stores it in the Builder's own field (FSTCompiler.java:352-355 at 5adff78bb168). A second build() on the same Builder therefore gives the new compiler the same buffer, which the first compile() already froze. The new compiler appends its bytes after the first FST's bytes but counts its node addresses from the start, so the second FST's root address points into the first FST. With a -> 1 in the first FST and b -> 2 in the second, the second FST returns null for b and 1 for a with assertions off; with assertions on, the second compile() fails on assert frozen == false in ReadWriteDataOutput.writeByte. Up to 9.9.2 each compiler made its own byte store, so the same calls gave two correct FSTs; #12624 (first released in 9.10.0) introduced the stored default. It is latent in Lucene itself: no Lucene module builds twice from one Builder.

1. First build(). dataOutput is null, so build() creates a ReadWriteDataOutput and assigns it to the Builder's own field: if (dataOutput == null) { dataOutput = getOnHeapReaderWriter(15); } (FSTCompiler.java:352-355). Compiler 1 writes its padding byte and its root node; the buffer holds 4 bytes, 00 01 61 1f. compile() records root address 3 and 4 bytes, and freezes the buffer (finish(), :960-965).
2. Second build(). The field is no longer null, so compiler 2 gets the same buffer object (a probe prints same output object: true).
3. Compiler 2 counts from 0. Every compiler starts its byte counter at 1, reserving address 0 for its padding byte (:172-175), whatever the buffer already holds. Its padding byte and root node really land at bytes 4-7: 00 01 61 1f 00 02 62 1f. Writes after freeze() are guarded only by assert frozen == false (ReadWriteDataOutput.java:52, :58): with assertions off they go through; with assertions on, the first one, the padding byte (writePaddingByte, FSTCompiler.java:576), throws inside the second compile().
4. FST 2 reads FST 1. FST 2's metadata says root address 3 and 4 bytes, the same values as FST 1's. getFSTReader() returns the whole shared buffer (:228-234), so FST 2's root is FST 1's root node: get(b) is null and get(a) is 1.
5. FST 1's save() grows. FST.save copies the whole buffer (FST.java:524-527, ReadWriteDataOutput.writeTo, :128-130): after the second build it writes 8 data bytes while its metadata still says 4. An independent probe with cat -> 5 and dog -> 7 measured 16 bytes against 8. Anything a caller writes after it in the same stream would be read at the wrong offset on reload (by reading; reload not executed).

Not claimed: a caller-supplied dataOutput shared across builds. That is the caller's choice; only the default, which the caller never chooses or supplies, is shared without their asking.

• Who is hit. Code that keeps one configured Builder without a dataOutput and builds several FSTs from it. Default settings, one key per FST and no scale are enough. The second FST is wrong whenever the first FST is non-empty and the second holds other non-empty keys, and a third build() is wrong the same way. Two cases happen to look right with assertions off: building the same keys twice, and an empty first FST (its compile() returns null before writing anything).
• Silent without assertions. With assertions disabled (the JVM default) there is no error: FST 2 misses its own keys and can answer FST 1's (get(a) is 1 in the probe). Only with assertions on does it fail, on an internal assert.
• Narrow reach. FSTCompiler is marked @lucene.experimental (FSTCompiler.java:68). All 53 new FSTCompiler.Builder sites in the tree build once; the three that keep a Builder variable (TestUtil, TestFSTs, FSTTester) also call build() once. Lucene's own analyzers, codecs and suggesters are not affected, and the recorded searches found no report since 9.10.0 (2024).
• Side effect. FST 1's save() afterwards also writes FST 2's bytes, after metadata that counts only its own.

TestFSTCompilerBuilderReuse (one method, extends LuceneTestCase, package org.apache.lucene.util.fst of the lucene/core tests). It builds FST 1 with a -> 1 and FST 2 with b -> 2 from the same Builder, using the FST.fromFSTReader(compile(), getFSTReader()) recipe from compile()'s javadoc, and asserts that FST 2 maps b to 2. The keys go in through IntsRefBuilder/IntsRef, which are marked @lucene.internal, exactly as in the FST package example (util/fst/package-info.java:47-50).

The test uses no randomness, so the seed does not matter (the hunt's earlier, longer version of the same scenario failed the same way with seeds 5EED0901, 5EED0902 and 7A3D9E11). As the regression test, the same method fits into TestFSTs as is.

cp $REPRO/TestFSTCompilerBuilderReuse.java $LUCENE/lucene/core/src/test/org/apache/lucene/util/fst/
cd $LUCENE && ./gradlew :lucene:core:test --tests org.apache.lucene.util.fst.TestFSTCompilerBuilderReuse -Ptests.seed=DEADBEEF
# and with -Ptests.asserts=false (assertions off)

# -Ptests.asserts=false
TestFSTCompilerBuilderReuse > testSecondBuildFromSameBuilder FAILED
    java.lang.AssertionError: expected:<2> but was:<null>
        at org.apache.lucene.util.fst.TestFSTCompilerBuilderReuse.testSecondBuildFromSameBuilder(TestFSTCompilerBuilderReuse.java:39)

# default (assertions on)
TestFSTCompilerBuilderReuse > testSecondBuildFromSameBuilder FAILED
    java.lang.AssertionError
        at org.apache.lucene.util.fst.ReadWriteDataOutput.writeByte(ReadWriteDataOutput.java:52)
        at org.apache.lucene.util.fst.FSTCompiler.writePaddingByte(FSTCompiler.java:576)
        at org.apache.lucene.util.fst.FSTCompiler.addNode(FSTCompiler.java:561)
        at org.apache.lucene.util.fst.FSTSuffixNodeCache.add(FSTSuffixNodeCache.java:154)
        at org.apache.lucene.util.fst.FSTCompiler.compileNode(FSTCompiler.java:388)
        at org.apache.lucene.util.fst.FSTCompiler.compile(FSTCompiler.java:1014)
        at org.apache.lucene.util.fst.TestFSTCompilerBuilderReuse.testSecondBuildFromSameBuilder(TestFSTCompilerBuilderReuse.java:36)

• Contract. build(): "Creates a new {@link FSTCompiler}." (FSTCompiler.java:350). The Builder is a "Fluent-style constructor for FST" (:237) whose setters return this. Nothing in the Builder, build() or dataOutput(...) javadoc (:301-315), CHANGES.txt or MIGRATE.md says a Builder is single-use, and nothing enforces it. Other Lucene APIs state single use explicitly, for example SegmentInfo.setCodec ("Can only be called once.", checked only by an assert, index/SegmentInfo.java:174-176) and PackedLongValues.Builder, which throws IllegalStateException("Cannot be reused after build()") on add() after build() (util/packed/PackedLongValues.java:237, :248, :267).
• History. Before #12624 the Builder's only storage setting was bytesPageBits (a page size, not a store), and each compiler made its own store (bytes = new BytesStore(bytesPageBits), FSTCompiler.java:134 at b6999818474^); 9.9.2 passes the same scenario. In #12624's review, an earlier revision created the default in a field initializer, and mikemccand asked "can we avoid even creating this, unless the caller didn't pass a Builder.dataOutput themselves?" (r1415507219, 2023-12-05). The creation moved into build() but still wrote the field. The thread is about the allocation; nobody discusses building twice. One earlier comment may come up: on a BytesStore revision mikemccand asked "Is caller allowed to freeze() more than once?" (r1391064549, 2023-11-13; no reply in the exported thread). It is about the output's freeze(), not Builder reuse, but with assertions off the second compile() here does call freeze() a second time on the same output.
• Controls. Two Builders, or one Builder given a fresh explicit output for each build, give two correct FSTs (cross-model review's control runs).

The argument. Builders are single-use by convention: the compiler itself is single-use (a second compile() on it returns null, because the first one cleared the root node, FSTCompiler.java:402, :1002-1005; and finish() refuses a second call with "already finished", :954-956), ReadWriteDataOutput's frozen assert shows one on-heap buffer is meant for exactly one FST, the API is @lucene.experimental, and no Lucene caller reuses a Builder.

Why it fails. The single-use object is the compiler, not the Builder, which holds only configuration. The frozen assert protects a buffer from a second writer, and it is build() that hands the buffer to one. Other Lucene APIs that mean single use say so; nothing says it here. And the same calls worked through 9.9.2.

What survives. A maintainer may prefer to document "one Builder per FST" (option C) rather than change the code; the code fix costs nothing for single use.

Option A restores the pre-9.10 behaviour (one buffer per compiler) and allocates exactly what a single build allocates today. Options B and C are policy choices for maintainers who prefer to declare the Builder single-use.

build() keeps the caller's dataOutput if one was set and otherwise creates a fresh on-heap output for this compiler only, without writing the Builder field.

     /** Creates a new {@link FSTCompiler}. */
     public FSTCompiler<T> build() {
-      // create a default DataOutput if not specified
-      if (dataOutput == null) {
-        dataOutput = getOnHeapReaderWriter(15);
-      }
+      // create a default DataOutput if not specified, a fresh one for every compiler
+      DataOutput out = dataOutput != null ? dataOutput : getOnHeapReaderWriter(15);
       return new FSTCompiler<>(
           inputType,
           suffixRAMLimitMB,
           outputs,
           allowFixedLengthArcs,
-          dataOutput,
+          out,
           directAddressingMaxOversizingFactor,
           version);
     }

Trade-offs

None for single use: it allocates the same one output per build() as today. A caller-supplied dataOutput is still passed to every compiler, as before (sharing it stays the caller's choice).

What the checks validated

• Gradle in a scratch worktree at 5adff78bb168, seed DEADBEEF: the repro fails without the fix (assertions on, and -Ptests.asserts=false) and passes with it (both).
• With the fix: :lucene:core:test --tests 'org.apache.lucene.util.fst.*' (9 classes, 38 tests, 5 skipped) and :lucene:suggest:test (31 classes, 236 tests, 7 skipped) pass with 0 failures; :lucene:core:checkGoogleJavaFormat passes.
• Independently: the cross-model reviewer ran TestFSTs with the same change (22 tests found; the 20 enabled ones pass; one skipped and one nightly-only test not run), and 9.10.0 with the same change passes the repro.

Throw IllegalStateException on a second build(), the way PackedLongValues.Builder rejects add() after build().

Trade-offs

Turns the silent wrong result into a loud error, but breaks code that worked through 9.9.2 and code that reuses a Builder with a fresh explicit output for each build.

What the checks validated

Not run.

State in the build() javadoc that a Builder without an explicit dataOutput must be built only once.

Trade-offs

No code change, but the misuse stays silent with assertions off. Can be added on top of A if maintainers want to state the intended use.

What the checks validated

Not run (documentation only).

The audit took the documented contract of FSTCompiler.Builder.build() and checked what state each call leaves behind. The default output is assigned to a Builder field rather than a local variable, so the obligation "a new compiler, independent of earlier ones" fails from the second call on.

An executable one-test repro witnessed it on main with assertions on and off. An independent reproducer, an executed cross-model review (OpenAI GPT-5.6, refute lens), a devil's-advocate verifier and an adjudication then confirmed it, and a version sweep placed its start in 9.10.0.