ydb-platform / ydb-java-sdk

YDB Java SDK
https://ydb.tech
Apache License 2.0
37 stars 21 forks source link

Fixing race condition in AsyncReaderImpl::shutdown #304

Closed Myllyenko closed 2 months ago

Myllyenko commented 2 months ago

This PR fixes a race condition that occurs upon invocation of AsyncReaderImpl::shutdown when the handlerExecutor field is set.

The problem we faced:

  1. User code calls AsyncReaderImpl::shutdown.
  2. Then following call chain follows: GrpcStreamRetrier::shutdownImpl -> AsyncReaderImpl::onShutdown -> AsyncReaderImpl::handleReaderClosed.
  3. handleReaderClosed sends an action calling eventHandler.onReaderClosed to handlerExecutor but does not wait for execution.
  4. AsyncReaderImpl::shutdown returns.
  5. User code shutdowns handlerExecutor.
  6. handlerExecutor rejects the action from the third step and eventHandler.onReaderClosed is never called.
codecov-commenter commented 2 months ago

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 46.37%. Comparing base (16f2600) to head (e741432).

Files Patch % Lines
...java/tech/ydb/topic/read/impl/AsyncReaderImpl.java 0.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #304 +/- ## ============================================ - Coverage 46.38% 46.37% -0.02% Complexity 1677 1677 ============================================ Files 303 303 Lines 12109 12108 -1 Branches 1208 1208 ============================================ - Hits 5617 5615 -2 - Misses 6054 6055 +1 Partials 438 438 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.