Skip to content

fix(s3stream): prevent closed reader from restoring blocks - #3555

Open
BackendArchitectX wants to merge 1 commit into
AutoMQ:mainfrom
BackendArchitectX:BackendArchitectX/test-streamreader-close-inflight
Open

BackendArchitectX wants to merge 1 commit into
AutoMQ:mainfrom
BackendArchitectX:BackendArchitectX/test-streamreader-close-inflight

Conversation

@BackendArchitectX

@BackendArchitectX BackendArchitectX commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #3615

Why

StreamReader.close() clears the current block state, but block-index/readahead work can still continue around the close lifecycle boundary.

There are two cases:

  1. An in-flight block-index/readahead load can complete after close().
  2. Queued or follow-up readahead work can start a new block-index load after close().

For work that started before close, the load can still see the same blocksEpoch and add blocks back into a reader that has already been closed.

For work that starts after close, epoch invalidation alone is not sufficient because the new load observes the updated epoch.

What

Prevent both in-flight and post-close block-index loads from restoring block state after a StreamReader is closed.

How

Increment blocksEpoch in close() before clearing the current blocks.

Existing block-index loading already checks the captured epoch before continuing and before inserting blocks, so changing the epoch prevents work started before close from restoring stale state.

Additionally, prevent loadMoreBlocksWithoutData0() from starting a new block-index load once the StreamReader is closed. This covers queued readahead and follow-up load continuations that begin after close.

Together, these checks ensure that a closed StreamReader cannot repopulate its block state through either existing in-flight work or newly started readahead work.

Verification

Regression behavior was validated against current main (7d4f733348).

Without the production fix:

  • testClosePreventsInflightReadaheadFromRestoringBlocks fails because asynchronous readahead restores blocksMap after close().
  • testClosedReaderDoesNotStartNewReadaheadLoad fails because ObjectManager#getObjects(...) is invoked after the reader has been closed.

With this PR applied to the same main revision:

  • both regression tests pass;
  • the complete StreamReaderTest passes;
  • :s3stream:checkstyleMain, :s3stream:checkstyleTest, and :s3stream:spotlessJavaCheck pass;
  • git diff --check passes.

The production change remains limited to four added lines.

AI Disclosure

Tool: ChatGPT — GPT-5.6 Sol.

AI was used for review and validation guidance around the lifecycle race and regression strategy. I reviewed the implementation and executed the reported validation locally.

@BackendArchitectX

BackendArchitectX commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor Author

Hi @Gezi-lzq — could you please take a look at #3555 when you have a chance?

The implementation changed materially after the original Aug 31 review request. The Sep 8 version added the closed guard needed to prevent queued/follow-up block-index loads after StreamReader.close(), in addition to epoch invalidation for already in-flight work.

I've now linked bug #3615 and validated both regressions against current main (7d4f733348): each fails without the production fix, while both and the complete StreamReaderTest pass with #3555 applied. Style and diff checks also pass.

The production change remains limited to four added lines.

Thanks.

@Gezi-lzq

Copy link
Copy Markdown
Contributor

Thanks for the heads-up! I'll take a look and review it as soon as I can.

@BackendArchitectX

Copy link
Copy Markdown
Contributor Author

Hi @Gezi-lzq

Just following up on #3555 whenever you have a chance.

Thank you again for taking a look at it. The PR is ready from my side and I’d really appreciate your review when convenient.

I’d be happy to address any feedback or changes you suggest.

Thank you for your time!

@BackendArchitectX

Copy link
Copy Markdown
Contributor Author

Hi @Gezi-lzq,

Hope you're doing well! Just wanted to kindly follow up on PR #3555. I understand you have a busy schedule and would really appreciate your review whenever you get a chance.

Happy to address any feedback or suggestions. Thanks again for your time and support!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Closed StreamReader can restore block state after close

2 participants