TeeSinkTokenFilter: a sink created before a stemmer is put on the tee throws on its first token

TeeSinkTokenFilter records the attribute state of every token it passes, and each sink made with newSinkTokenStream() replays those states later, so that several fields can share analysis steps. The sink is built from cloneAttributes(), a copy of the attribute implementations the tee has at that moment. A filter put on the tee afterwards, the order of the class javadoc's example, shares the tee's attributes; when its constructor adds a new one, for example PorterStemFilter adding KeywordAttribute, every state the tee records contains it, the sink does not, and the sink's first restoreState() throws IllegalArgumentException. With IndexWriter, the tee's field first as documented, every document that uses the chain is rejected (5 of 5 texts tried, from an empty value to 2,000 tokens; every recorded state carries the attribute, including the end state that even an empty value has, so the text cannot matter); the exception does not abort the writer and CheckIndex stays clean. Creating the sink after the stemmer, or adding the attribute to the sink by hand, avoids it. The javadoc's own LowerCaseFilter adds no new attribute implementation, so its example works; a narrow split setup fails even with it (a payload-storing MemoryIndex reads the tee while something else reads the sink). The snapshot dates from 2009 (2.9.0); a correct 2011 fix of a stale state cache (LUCENE-3042) exposed it, so every release from 3.2.0 to 10.6.0 and main fails.

1. newSinkTokenStream() returns new SinkTokenStream(this.cloneAttributes(), cachedStates) (TeeSinkTokenFilter.java:68-70). cloneAttributes() clones the attribute implementations present at that moment (AttributeSource.java:424-431); the sink never looks at the tee's attributes again.
2. A filter wrapped around the tee shares the tee's attribute maps: TokenFilter(input) calls super(input) (TokenFilter.java:34-37), and AttributeSource(AttributeSource) "uses the same attributes as the supplied one" (AttributeSource.java:78-85). PorterStemFilter adds KeywordAttribute when it is built (PorterStemFilter.java:54), as TokenStream asks ("the attributes must be added during instantiation", TokenStream.java:64-66). Now the tee has a KeywordAttributeImpl; the sink's copy does not.
3. The tee records captureState() for every token (TeeSinkTokenFilter.java:85) and at end() (:95). A state holds every implementation of the shared source (AttributeSource.java:300-303), so it includes KeywordAttributeImpl.
4. SinkTokenStream.incrementToken() and end() call restoreState(state) (TeeSinkTokenFilter.java:121, :129). restoreState looks up each implementation of the state by class in the sink and throws IllegalArgumentException when one is missing (AttributeSource.java:322-327): the first token fails; a value without tokens fails in end().
5. In IndexWriter the exception escapes stream.incrementToken() (IndexingChain.java:1987); analyzer exceptions are non-aborting (comment :1989-1994) and the document is marked deleted (DocumentsWriterPerThread.java:303-306).

Default tokenizers start with one packed implementation of term, offsets, position increment and length, type and term frequency, so those never trigger it; StopFilter stopped triggering it in 4.9.0 for that reason.

Before LUCENE-3042 (Uwe Schindler, 2011) each filter of a chain kept its own cached state, so the tee's captureState() returned stale states without the late attribute and the sink replayed them without error (for the stemmer this matters only in 3.1.0: in 2.9.x and 3.0.x PorterStemFilter adds no KeywordAttribute yet). LUCENE-3042 correctly made the chain share one cache, which exposed the 2009 snapshot. Swapping only AttributeSource.java flips the result both ways: 3.1.0 with the new file fails 3 of 3 cases, 3.2.0 with the old file passes 3 of 3.

• Who is hit. Code that builds the chain in Java in the javadoc's order with a filter on the tee that adds an attribute implementation the tee did not have yet. A plausible use is a stemmed field next to an unstemmed copy (PorterStemFilter, KStemFilter, SnowballFilter, ...); payload (DelimitedPayloadTokenFilter) and flags (TypeAsSynonymFilter) filters do the same. Narrowly also a split setup: a MemoryIndex with storePayloads=true reads the tee's field and a consumer that does not add PayloadAttribute reads the sink, which fails even with LowerCaseFilter; one such MemoryIndex for both fields works.
• Default vs opt-in. Opt-in and expert: TeeSinkTokenFilter has no factory (TestAllAnalyzersHaveFactories exempts it), so analysis configuration files cannot use it; a code search found no use in the major open-source search servers built on Lucene (default branches).
• What happens. With the tee's field first (the documented order), every document built with the chain is rejected with IllegalArgumentException: 5 of 5 texts tried (empty, whitespace only, one token, a sentence, 2,000 tokens), and by the code the text cannot matter. The writer stays open, no tragic exception is recorded, nothing of the rejected document is searchable, later documents index normally, and CheckIndex is clean.
• Scale and likelihood. Deterministic from the first document; independent of text and index size. Plausible among the few users of the class: it follows the documented order, but it fails at once and loudly, and the recorded public searches found no report of it since 3.2.0 (2011).
• Workarounds. Create the sinks after the tee's whole chain is built, or call sink.addAttribute(...) for each attribute the tee's chain adds (KeywordAttribute.class for stemmers).

Copy the file to lucene/analysis/common/src/test/org/apache/lucene/analysis/sinks/. It sends one token, foxes, through WhitespaceTokenizer -> TeeSinkTokenFilter, creates the sink, then wraps the tee in PorterStemFilter, consumes the stemmed stream and reads the sink. Deterministic: no randomness, the seed only fixes the runner. It reads both streams by hand on purpose: assertTokenStreamContents adds a check attribute of its own to every stream it reads, which would be a second late attribute (in this order the sink throws for KeywordAttributeImpl either way). Name tests.asserts explicitly: the build randomizes it per seed otherwise, and DEADBEEF picks off. Through IndexWriter the same exception rejects the document in addDocument; that is described, not tested here.

TestTeeSinkAttributeAddedAfterSink > testSinkAfterStemmerOnTee FAILED
    java.lang.AssertionError: expected the sink to replay "foxes", but it threw java.lang.IllegalArgumentException: State contains AttributeImpl of type org.apache.lucene.analysis.tokenattributes.KeywordAttributeImpl that is not in in this AttributeSource
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:FE21949B5686915D]:0)
        at org.apache.lucene.analysis.sinks.TestTeeSinkAttributeAddedAfterSink.testSinkAfterStemmerOnTee(TestTeeSinkAttributeAddedAfterSink.java:30)

        Caused by:
        java.lang.IllegalArgumentException: State contains AttributeImpl of type org.apache.lucene.analysis.tokenattributes.KeywordAttributeImpl that is not in in this AttributeSource
            at org.apache.lucene.core@11.0.0-SNAPSHOT/org.apache.lucene.util.AttributeSource.restoreState(AttributeSource.java:326)
            at org.apache.lucene.analysis.sinks.TeeSinkTokenFilter$SinkTokenStream.incrementToken(TeeSinkTokenFilter.java:121)
            at org.apache.lucene.analysis.sinks.TestTeeSinkAttributeAddedAfterSink.testSinkAfterStemmerOnTee(TestTeeSinkAttributeAddedAfterSink.java:28)

1 test completed, 1 failed

# the same with -Ptests.asserts=false and on the 10.6.0 release jars; with the recommended fix the test passes

• The documented order fails once the filter on the tee adds a new attribute. The example creates sink1 and sink2 and then writes TokenStream final1 = new LowerCaseFilter(source1); (TeeSinkTokenFilter.java:36-40); the class is pitched for fields that "share many common analysis steps and then go their separate ways" (:28-30). The only documented constraint is that "tees are consumed before sinks" (:53-57).
• The promise. newSinkTokenStream(): "Returns a new SinkTokenStream that receives all tokens consumed by this stream" (:67).
• The design intent. In LUCENE-1693 (#2767, 2009), which added the class, Michael Busch wrote that the factory method is the way to "ensure that the sink tokenstream has exactly the same attributes as the TeeSinkTokenFilter". Related history, not a precedent for replaying: the pre-5.5 addSinkTokenStream(), which attached an existing sink to a second tee, had a loop to "add eventually missing attribute impls to the existing sink" when it was attached (removed with that method by LUCENE-6973, first released in 5.5.0).
• The stemmer follows the rules. TokenStream: "the attributes must be added during instantiation" (TokenStream.java:64-66).
• A similar class states and checks its precondition. ConcatenatingTokenStream: "All of the input TokenStreams must have the same attribute implementations" (ConcatenatingTokenStream.java:36), checked when it is built, with "Attempted to concatenate TokenStreams with different attribute types" (:68-83). No TeeSinkTokenFilter javadoc, CHANGES or MIGRATE entry mentions an attribute or creation-order limit, and no test asserts the exception.
• Precedent. SOLR-8460 (2015) fixed the same kind of bug in Solr's ListBasedTokenStream by adding the source's attribute implementations.

The strongest case for "works as designed" (rated moderate by the cross-model reviews; the devil's-advocate verifier built the same case and rejected it): cloneAttributes() is documented as a copy taken when it is called (AttributeSource.java:424-430), restoreState() says the target "must contain a corresponding instance for each argument contained in this state" (:305-311), TokenStream asks for attributes at instantiation, which could be read as "build the whole chain before deriving sinks", and upstream's TestTeeSinkTokenFilter.testGeneral adds its late check attribute to the sink by hand (TestTeeSinkTokenFilter.java:121-124, since LUCENE-2211 in 2010), so the authors knew. In 2009 on dev@ ("attribute thoughts"), asked by Yonik Seeley whether filters may add attributes after their constructor, Michael Busch said the TokenStream javadocs ask for all attributes up front ("We could change these semantics"); asked whether restoreState() should be able to add attributes, he said that could only work if both sources use the same AttributeFactory, and later agreed with Yonik Seeley's suggestion of an assertion that the factories are equal.

Why it fails: the class's own example creates the sinks first and then wraps the tee, and newSinkTokenStream() promises a sink that "receives all tokens consumed by this stream"; the snapshot and restoreState() rules are internal mechanics that TeeSinkTokenFilter itself has to reconcile, not a contract stated to its users; the stemmer adds its attribute at instantiation, as required; and the test workaround is unexplained test code (and redundant, because assertTokenStreamContents adds that attribute itself, BaseTokenStreamTestCase.java:162-163). A narrower, secondary point: when a payload-storing MemoryIndex reads the tee and something else reads the sink, the consumer adds the attribute, so "build the chain first" does not help there. Verdict: not intended, a usability and correctness defect with a loud failure mode. Maintainers may still prefer to fix it in the javadoc.

Option A was applied in a scratch worktree of 645d4fe14a20 and run with gradle (the worktree was removed after the runs); it is the finder's fix with a renamed method and comments. Options B and C were not built. The sibling LFV-651 (a sink never clears attributes only its own chain adds) changes other lines of the same class; the two patches were not tested together.

The sink keeps a reference to the tee (newSinkTokenStream() passes this), and reset() and end() call addTeeAttributes() before replaying. addAttributeImpl returns at once for an implementation class the sink already has (AttributeSource.java:188-190), so existing attributes, including ones the sink's own chain added, are untouched; a new copy holds the tee's current values until the next restoreState overwrites them. Scope limit: addAttributeImpl registers a new class only for an interface the sink does not have yet (:193-200), so an implementation class whose interfaces the sink already has through other classes is not added and restoreState still throws; with the shared AttributeFactory this needs custom implementations added by hand. An attribute the tee gains after the sink's reset() is added only at the sink's end(), before the final state is restored; a token state replayed before that which contains it still throws. That needs the tee to be read after the sink's reset(), which is outside the documented order (tees consumed before sinks).

private void addTeeAttributes() {
  for (Iterator<AttributeImpl> impls = tee.getAttributeImplsIterator(); impls.hasNext(); ) {
    // addAttributeImpl does nothing for an implementation this sink already has
    addAttributeImpl(impls.next().clone());
  }
}

@Override
public void reset() {
  addTeeAttributes();
  it = cachedStates.getStates();
}
// end(): the same call before restoreState(finalState)

Trade-offs

• Fixes every tested trigger, including the split MemoryIndex(storePayloads=true) case that a javadoc note cannot cover.
• The sink's attribute set can now grow at reset() and end(). In the setups that work today the sink normally lacks nothing, so nothing is added; where something is added, the extra implementations (holding the tee's current values until the next restoreState) are visible through the sink's AttributeSource, e.g. hasAttribute or getAttributeImplsIterator, and no test covers code that inspects them. A sink reused across documents keeps the grown set; reviewers may ask about this.
• Cost: one clone() per tee attribute implementation per reset() and end(), also for ones the sink already has (the copy is then discarded); a handful of small objects per sink and document, not benchmarked.
• Not fully general: custom implementation classes added by hand with addAttributeImpl are not covered, and an attribute the tee gains after the sink's reset() is added only at end(), so token states replayed before that still throw (only outside the documented order).
• Backport: on 10.6.0 the sink's incrementToken() and reset() are declared final, so the reset() hunk needs a manual edit; only main was built with the fix. Not validated: another JDK or architecture, custom implementations whose interfaces overlap, or together with the LFV-651 fix.

What the checks validated

• Gradle in a scratch worktree of 645d4fe14a20, stock: the repro fails with assertions on and off (1 test, 1 failure each).
• With the fix applied: the repro and TestTeeSinkTokenFilter (3 upstream tests) pass with assertions on and off (4 tests, 0 failures); the whole :lucene:analysis:common test task passes with assertions on: 340 suites, 1,910 tests, 0 failures, 18 skipped.
• Earlier runs of the same patch: the whole analysis-common test task with assertions off and :lucene:analysis:common:check -x test (format, ecjLint, forbiddenApis) passed; shadowed into the stock main jars (the patched class compiled ahead of them, not a built Lucene), the finder's 2 tests (incl. the IndexWriter variant) and the independent reproducer's 6 tests pass, and the devil's-advocate probe no longer throws for the stemmer or the MemoryIndex(storePayloads=true) case.
• With the finder's patch (same logic): the cross-model reviewer's 5 tests pass, and the exhaustive model check has 0 mismatches (4,092 of 4,092 inputs fail on stock).

State in the class javadoc that sinks must be created after the tee's whole chain is built, and reorder the example (and fix its declared sink type: TeeSinkTokenFilter.SinkTokenStream sink1 = source1.newSinkTokenStream(); does not compile, the method returns TokenStream).

Trade-offs

No code change, but existing code that follows the current example's order with a filter that adds a new attribute keeps failing, and it does not help in the narrow split case where the tee's consumer adds an attribute (a payload-storing MemoryIndex reads the tee, something else reads the sink).

In reset(), check that the sink has every implementation of the tee and throw an IllegalStateException saying "create sinks after the tee's chain is built".

Trade-offs

A clearer and earlier error (at the sink's reset()), and the javadoc's LowerCaseFilter example keeps working, but the documented order with a filter that adds a new attribute still fails. It would also catch the MemoryIndex case: MemoryIndex adds PayloadAttribute before it resets the tee (MemoryIndex.java:1006-1008), so the tee already has it when the sink is reset later.

Method: semi-formal (bounded-exhaustive translation validation against reference semantics).

• The model states the javadoc's promise: every sink replays exactly the tee's token states and its end state. It was run against the stock TeeSinkTokenFilter over 0 to 4 input tokens x position increments {1, 2} x offset gaps {0, 1} x final increment {0, 1} x final offset slack {0, 1} x 1 to 3 sinks, in five consumption scenarios of 4,092 checks each.
• The scenario "a filter that adds an attribute is put on the tee after the sinks were created" failed on all 4,092 inputs with this exception; "tee consumed first", "consumeAllTokens() plus end()" and "a sink read twice" had 0 mismatches. A fifth scenario, consumeAllTokens() without the tee's end(), had 3,579 mismatches (a stale end state, the same with the fix); its sibling report was withdrawn as works-as-designed.
• The finder reduced it to a public-API JUnit with a stemmer and an IndexWriter variant, later minimized to the one-token test on this page.