DirectIODirectory: after a read throws, the next seek or read on the same input can return bytes of another file position

DirectIODirectory (lucene/misc) reads a file through a 256 KiB buffer window. Its DirectIOIndexInput.fill() first moves the window address filePos to the next block, then checks for EOF, then clears the buffer and reads the channel. When the EOF check throws (a read asked for more bytes than are left) or the channel read throws (a storage error, a closed channel, an interrupt), nothing is rolled back: filePos names the next block while the buffer keeps the previous block's bytes (or nothing yet, on a fresh input). getFilePointer(), seek()'s in-buffer shortcut and the read fast paths all trust that pairing, so the next seek or read on the same input silently returns bytes from exactly one window (262,144 bytes) away, or zeros, or throws EOFException at a legal position. In the repro, after a read past EOF on a 262,240-byte file, seek(262148) and readInt() return 4 instead of 262,148. It is in every release since 9.0.0 (the class's first) and on main; 10.2.0 widened it. Lucene's own code stops at the first exception, so it takes application code that keeps using the input.

• DirectIODirectory: an opt-in Directory in lucene/misc that reads and writes with O_DIRECT, bypassing the OS page cache. By default it does so only in a merge context when the merge's estimated size (if known) and, for reads, the file are at least 10 MiB; everything else goes to the wrapped FSDirectory.
• Window: each DirectIOIndexInput reads through its own direct buffer of 262,144 bytes (DEFAULT_MERGE_BUFFER_SIZE). filePos is the file offset of the buffer's first byte, and getFilePointer() is filePos + buffer.position() - offset (DirectIODirectory.java:388).

1. fill() (:496-516) does filePos += buffer.capacity() first (:497). Its comment says it throws EOFException early "to maintain buffer state (position in particular)" (:499-500), but the check (:501-503) runs after the move. Then buffer.clear() (:505) and channel.read(buffer, filePos) (:510); a failed read is rethrown with nothing restored (:511-513).
2. After an EOF throw, the buffer still holds the previous block with its old limit (on a fresh input it is still empty). After a failed read, it is cleared (position 0, limit 262,144) but its memory still holds the previous block.
3. seek() does nothing when pos == getFilePointer() (:403) and repositions inside the buffer when the target lies in [filePos, filePos + limit] (:413-416); readByte and readBytes serve straight from the buffer while it has bytes, readShort/Int/Long while it has at least 2, 4 or 8 (:444-477, :519-533).

Every file stores its own position as an int at each multiple of 4.

Step by step for D: after window 0, filePos is 0 and the buffer holds bytes 0-262,143. fill(200) moves filePos to 262,144 and throws, so getFilePointer() becomes 524,288. seek(262148) finds 262,148 in [262,144, 524,288] and sets position 4 without reading; 262,140 bytes remain, so readInt() returns bytes 4-7 of the old window. For B, the throwing first fill leaves filePos at 0 with an empty buffer, so seek(0) is a no-op and the next fill moves on to the second window.

Also executed (the 7-trigger history test): the same as D on a slice, a clone read after the original was closed, and a read after a read interrupted by Thread.interrupt (the last two are close to misuse). Run for this doc: a seek whose channel read fails (seek(270340) on a 524,288-byte file after window 0) leaves getFilePointer() at the block start 270,336 while the next readInt() returns 0 and the one after it 4, both from window 0. The wrong bytes always come from the same input's own buffer: one window away in the same physical file, bytes just outside a slice's range (shown after a seek past the slice's end), or zeros.

• Who is hit. Users of DirectIODirectory (@lucene.experimental, DirectIODirectory.java:53-61). By default it uses direct I/O for reads in a merge context when the merge's estimated size (if known) and the file's length are both at least minBytesDirect, 10 MiB by default (useDirectIO, :168-173; :72) and for every read an application routes to it by overriding useDirectIO. Within the class, default settings suffice: the stock-policy row of the table above.
• When. Code that keeps using the same input after a read on it threw: a retry after a transient storage error, a read after probing with a too-large read, a custom codec or file tool that catches EOFException and goes on. It then silently gets bytes of another position while getFilePointer() reports the right one, or a spurious EOFException.
• Not in Lucene's own paths. Merges abort on the first exception; SegmentInfos (:314) and SegmentCoreReaders (:141) turn EOFException into CorruptIndexException; CodecUtil.checkFooter(in, priorException) reads on after an exception, but on checksum inputs, which open with IOContext.READONCE (Directory.java:155-156) and so go to the delegate. No in-tree wrong result was shown.
• Scale. None needed: one exception, then one read. Wrong bytes appear only on the input whose read threw: in the security probe (state provoked by a seek past the end), clones taken before and after the failure and a new slice read the right bytes. All views of a file share one FileChannel (:354), so after a close or an interrupt none of them can read new blocks.
• Versions. Every release from 9.0.0 to 10.6.0 (10.6.0 run at its RC1 revision) and main; 10.2.0 added the legal-seek trigger and the closed-channel triggers.

# Lucene main 645d4fe14a20, assertions on (-ea), seed DEADBEEF: 1 test(s) run, 1 failed
    => java.lang.AssertionError: expected:<81> but was:<1>
       org.junit.Assert.fail(Assert.java:89)
       org.junit.Assert.failNotEquals(Assert.java:835)
       org.junit.Assert.assertEquals(Assert.java:647)
       org.junit.Assert.assertEquals(Assert.java:633)
       org.apache.lucene.misc.store.TestDirectIOSeekAfterReadPastEOF.testLegalSeekAfterReadPastEOF(TestDirectIOSeekAfterReadPastEOF.java:45)

• IndexInput#getFilePointer (IndexInput.java:61-66): "Returns the current position in this file, where the next read will occur." IndexInput#seek (:68-75): "Sets current position in this file, where the next read will occur. If this is beyond the end of the file then this will throw EOFException and then the stream is in an undetermined state." Triggers A, B, C and D never seek past the end.
• DataInput#readBytes (DataInput.java:45-52) and the other read methods say nothing about the state after they throw, so the argument rests on what follows: in B, C and D a legal seek, after which the javadoc says where the next read occurs; in A, getFilePointer() naming a position whose bytes are not the ones returned.
• fill()'s comment (DirectIODirectory.java:499-500): it throws EOFException early "to maintain buffer state (position in particular)", citing BaseDirectoryTestCase#testSeekPastEOF (consecutive reads past EOF). The early throw does keep the buffer's position, but the filePos advance before it, which keeps reads after a seek past EOF throwing (testSeekPastEOFAndRead), also runs on the read-past-EOF and failed-read paths, where it puts filePos out of step with the buffer.
• IndexInput class javadoc (:37-38): reads on a clone after the original is closed "will throw AlreadyClosedException"; the clone trigger returns stale bytes instead.
• 2021 review of LUCENE-9686 (apache/lucene-solr#2258), a review of the post-EOF semantics. dweiss: "I wonder if we should move the channel's position to actually point after the last byte, then throw EOFException... so that not only we indicate an EOF but also leave the channel pointing at the end. I have a scenario in my mind when somebody tries to read a bulk of bytes, hits an eof but then a single-byte read() succeeds. That would be awkward, wouldn't it?", and in the PR conversation "I have some doubts whether EOF should leave the channel undrained." zacharymorn answered that the class "already handles that actually (by manipulating filePos, but not channel.position per se)" and added seek-past-EOF tests; dweiss: "Ok. Thanks for explaining!". The reviewer preferred drain-to-end (the next read fails); the exchange settled on "filePos keeps the state consistent", and nobody noticed that filePos then stops matching the buffer. On main the case dweiss called awkward does happen, with wrong bytes (B without the seek). The recommended fix keeps the other semantics (see Fixes).
• Precedents: LUCENE-1262 (#2339, 2008), stale bytes in BufferedIndexInput after an exception in readInternal, and LUCENE-2852 (2011), RAMInputStream "hits false EOF if you seek to EOF then seek back then readBytes", both fixed as bugs.
• The reference, MMapDirectory, returns the right bytes for B, C, D and the stock-policy steps; A has no reference run.

The strongest case for "intended": the class is @lucene.experimental and warns it "quite easily could contain horrible bugs ... Use at your own risk" (DirectIODirectory.java:53-56); no read method promises a usable state after it throws, and IndexInput#seek leaves the state undetermined after a seek past the end; merges abort on the first exception, so nobody reads on; the filePos advance before the EOF check is load-bearing (it keeps reads after a seek past EOF throwing, and Elasticsearch's copy keeps it too); the 2021 review accepted the post-EOF handling; and #16619, whose description keeps "the behaviour identical to today, quirks included", added "a characterization test, not an endorsement" of a different post-EOF quirk (TestDirectIODirectory.java:380-387).

Why it fails: "experimental" lowers priority but documents no wrong bytes; the main triggers involve no seek past the end and end with a legal seek whose result the javadoc defines (the finder's two seek-past-EOF tests were dropped for that reason); the fill() comment shows the intent to keep the buffer state, and the 2021 review accepted the class on the answer that filePos keeps the state consistent, which is what fails here; what the advance does for a seek past EOF (later reads keep throwing) can be kept while the read paths stop desynchronizing (the recommended fix does both); LUCENE-1262 and LUCENE-2852 were treated as bugs; and the #16619 quirk (a read past a slice's own length served from the same window) is a different behaviour that the fix keeps. The adjudication rated the case for intended behaviour as failing; the cross-model reviewers rated it moderate.

One fix was built and validated with gradle in a scratch worktree at a9779ef3c193; the finder's earlier sketch was validated too and has two gaps. The other options are untested.

DirectIODirectory.java only, +48/-11. Three parts: (1) fill() checks EOF against the next block before changing any state, and after a failed channel read empties the buffer (limit(0)) and sets a flag so that the next fill reads the same block again; (2) seekInternal(), on EOFException (only a seek past the end gets there), leaves the input at its end with an empty buffer, so later reads keep throwing EOFException and no later seek lands in the old window; (3) fillPending(), the lazy first fill of a clone or slice (#16619), restores the pending start if its read fails. The excerpt below is condensed (the diff keeps and extends the original comments; the logic is the same).

private void fill(int bytesToRead) throws IOException {
  // the next block, or the same block again after its read failed
  final long nextFilePos = refillSameBlock ? filePos : filePos + buffer.capacity();
  // throw EOFException before any state changes: filePos must keep matching the buffer
  if (nextFilePos > offset + length || ((offset + length) - nextFilePos < bytesToRead)) {
    throw new EOFException("read past EOF: " + this);
  }
  filePos = nextFilePos;
  refillSameBlock = false;
  buffer.clear();
  try {
    channel.read(buffer, filePos);
  } catch (IOException ioe) {
    buffer.limit(0); // nothing valid is buffered: read this block again next time
    refillSameBlock = true;
    throw new IOException(ioe.getMessage() + ": " + this, ioe);
  }
  buffer.flip();
}

Trade-offs

• Only the state after an exception changes; success paths do one extra boolean test per 256 KiB fill, and readByte and the other read fast paths are untouched. No benchmark was run.
• New, deterministic behaviour after a failure, in which filePos always matches the buffer. After a read past EOF, getFilePointer() is where the bytes the read consumed end and the next read continues there with the right bytes: in B, C and D where the failed read started, as with MMapDirectory; if the buffer still had bytes, readBytes copies them out first, so the pointer ends at the window's end. After a failed channel read the next read retries the same block; after a seek past EOF getFilePointer() equals length(). The read is not made atomic.
• Its post-EOF semantics are not the drain-to-end ones dweiss suggested in the 2021 review: a single-byte read after a failed bulk read succeeds, with the right bytes. A drain-to-end variant (on a read past EOF, set filePos to the end and empty the buffer, as part (2) does for a seek past EOF) would, by reading, also fix B, C and D; untested.
• A seek whose channel read fails leaves the input at the start of the target's block, not at the target: getFilePointer() reports the block start, the next read returns the bytes there, and seeking again works (executed: 270,336 then 270,340; stock main returns 0 and 4). That probe belongs in the regression tests.
• Neither stock nor this fix loops on a short channel.read (the FileChannel contract allows fewer bytes than requested); the next fill would then skip the unread bytes. A separate question, untested.
• #16619's characterization tests still pass, so that quirk (a read past a slice's own length served from the window) is unchanged.
• A clone read after the original was closed now keeps throwing IOException, still not the AlreadyClosedException that IndexInput's javadoc promises; that gap is unchanged.
• Backport: branch_10x and branch_10_6 have refill() with the same ordering but no lazy slice start, so they need parts (1) and (2) only (read). Add a CHANGES entry under Bug Fixes.

What the checks validated

• Clean scratch worktree at a9779ef3c193, gradle. Stock: the repro (5), the 7-trigger history test and the 3-scenario probe fail 15 of 15. With the fix: all 16 pass (the probe now with 4 tests), assertions on and off, and every probe scenario that has a reference line matches it.
• Closest suites with the fix: TestDirectIODirectory (69 tests, 2 skipped by their own FSDirectory-only assumptions) and TestDirectIODirectoryCopyFrom (1), assertions on and off; the whole lucene/misc module, its 23 test classes (315 tests) plus the 3 evidence classes (16 tests): 331 tests, 0 failures, 9 skipped.
• checkGoogleJavaFormat, ecjLintMain, ecjLintTest, forbiddenApisMain and forbiddenApisTest of lucene/misc pass with the fix and the repro test in place. The worktree was removed afterwards.
• The same class shadowed onto the stock jars, assertions on and off: the finder's 5 tests, the independent reproducer's 6 and the devil's-advocate read-past-EOF probe (5) pass; in the transient-error probe the DirectIO test passes and the NIOFS control still fails, which is the separate core bug LFV-492.
• The failed-seek probe passes with the fix, assertions on and off; the independent fact check re-ran the fix through gradle with fresh seeds (the repro, TestDirectIODirectory, TestDirectIODirectoryCopyFrom: pass).

+40/-5: on a failed fill set the buffer's limit to 0 and an "invalid" flag that disables seek()'s shortcut; after a failed channel read, do not advance filePos again.

Trade-offs

It keeps filePos moved after a read past EOF, so the next read without a seek still skips a window: after B's failed read readInt() returns 262,144 at file pointer 0, and after D's it throws EOFException with 96 bytes left. After a seek past the end of slice(100, 4) of a 200-byte file, getFilePointer() fails its own assertion under -ea (and returns 0 under -da).

What the checks validated

The repro and history tests pass (12 of 12), and so do TestDirectIODirectory (69) and TestDirectIODirectoryCopyFrom (1); 2 of the 3 probe scenarios still differ from the reference.

A few lines.

Trade-offs

Fixes the read-past-EOF triggers B, C and D, but not the failed-read, closed-channel or interrupt triggers. By reading, it also breaks TestDirectIODirectory.testSeekPastEOFAndRead (a read after a seek past EOF must throw) unless seekInternal() handles the seek past EOF as in the recommended option. Untested.

Every later read and seek throws.

Trade-offs

Simple and safe. No contract promises that an input stays usable after a read throws, so poisoning after a low-level I/O failure is defensible; after an ordinary read past EOF it is stricter than MMapDirectory, which keeps reading the right bytes there. Untested.

Method: cross-implementation differential testing against an independent reference model of IndexInput (the hunt's "semi-formal" tier; no model checker, solver or prover found it).

• The harness drove 29 IndexInput configurations over byte-identical files with the same operation sequences: 225.6 million sequences and 559 million checks over those 29 configurations, with assertions on and off, on the stock main jars.
• After the documented differences were classified, 21 mismatches were left, all post-EOF recovery on DirectIODirectory: provoke an EOFException, seek back to a legal position, read, and get a wrong byte or a spurious EOFException. Its close-semantics and interrupt phases showed the failed-read variant.
• Reading fill() located the ordering, and the finding was reduced to public-API JUnit tests. With the fix sketch the harness found 0 wrong-value and 0 spurious-exception mismatches; the documented-difference buckets remain.