StandardTokenizer.close() can leave the object in an uncloseable state [LUCENE-6656]
- Dominant language
- Java
- Stars
- 3.6k
- Forks
- 1.4k
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 88
Description
The following pair of tests shows that if a reader throws IOException from the close() method, StandardTokenizer is left in an inconsistent state where it thinks you didn't call close on the tokeniser, even though you did. To make matters worse, it holds onto the reader so that any subsequent attempts to close the tokeniser will also fail.
Possible workarounds:
1. Don't reuse tokenisers.
2. Still reuse tokenisers, but if close() throws anything, discard that tokeniser and create a new one.
3. Wrap every reader you pass in to ensure that close() can't throw an exception.
Code follows:
```Java
public class TestStandardTokenizerCloseIssue {
`@Test`
public void testStreamReuse() throws Exception
{
// Attempts to verify that consumeAndClose itself is not broken.
try (Tokenizer stream = new StandardTokenizer())
{
stream.setReader(new StringReader("reader #1"));
assertThat(consumeAndClose(stream), contains("reader", "1"));
stream.setReader(new StringReader("reader 2"));
assertThat(consumeAndClose(stream), contains("reader", "2"));
}
}
`@Test`
public void testStreamReuseAfterFailure() throws Exception {
class FailingReader extends Reader {
`@Override`
public int read(`@NotNull` char[] buffer, int off, int len)
throws IOException {
throw new IOException("Synthetic exception");
}
`@Override`
public void close() throws IOException {
throw new IOException("Synthetic exception");
}
}
// Simulating sharing the instance inside some factory.
try (Tokenizer stream = new StandardTokenizer()) {
try {
stream.setReader(new FailingReader());
consumeAndClose(stream);
fail("Expected IOException");
} catch (IOException e) {
// Expected
}
stream.setReader(new StringReader("working reader"));
// Test fails here - even though the consumeAndClose above
// did close the tokeniser, the tokeniser didn't clear its reference to
// the reader.
assertThat(consumeAndClose(stream), contains("working", "reader"));
}
}
// Attempts to implement the correct workflow for consuming a
// TokenStream.
private List consumeAndClose(TokenStream stream)
throws Exception {
ImmutableList.Builder tokens = ImmutableList.builder();
//The consumer calls reset().
stream.reset();
try {
// The consumer retrieves attributes from the stream and stores
// local references to all attributes it wants to access.
CharTermAttribute termAttribute =
stream.getAttribute(CharTermAttribute.class);
// The consumer calls incrementToken() until it returns false
// consuming the attributes after each call.
while (stream.incrementToken()) {
tokens.add(termAttribute.toString());
}
// The consumer calls end() so that any end-of-stream operations
// can be performed.
stream.end();
} finally {
// The consumer calls close() to release any resource when finished
// using the TokenStream.
stream.close();
}
return tokens.build();
}
}
```
Originally discovered on 4.10.4. Code has been ported to work on 5.1 since initially created and sooner or later I'll get to test 5.2.1, but I don't see anyone else having reported a similar issue yet, so I'm guessing it won't be fixed yet.
---
Migrated from [LUCENE-6656](https://issues.apache.org/jira/browse/LUCENE-6656) by Trejkaz
Contributor guide
Research direction
Start at StandardTokenizer.close() and reproduce the failure using the supplied TestStandardTokenizerCloseIssue tests, especially testStreamReuseAfterFailure with FailingReader. Trace how the reader reference and close state are handled after IOException. Done means the tokenizer can be reused after a failed close and the supplied reuse assertions pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- search
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100