Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix deadlock and couple more problems in DefaultConnectionPool #699

Merged
merged 4 commits into from May 1, 2021

Commits on Apr 25, 2021

  1. Configuration menu
    Copy the full SHA
    ec11550 View commit details
    Browse the repository at this point in the history
  2. Fix deadlock in DefaultConnectionPool

    This commit fixes two problems in the `OpenConcurrencyLimiter`:
    - deadlock in `DefaultConnectionPool`;
    - potential removal from `OpenConcurrencyLimiter.desiredConnectionSlots` when its empty.
    
    Deadlock
    
    a) Before introducing OpenConcurrencyLimiter, "AsyncGetter" thread was used only
    to do blocking `ConcurrentPool.get`.
    b) After, I started to additionally use "AsyncGetter" to do
    blocking `OpenConcurrencyLimiter.waitUntilOpenPermitAvailable`.
    
    As a result, we may have a thread that gets the last connection (`maxSize` is reached)
    from `ConcurrentPool` and submits `waitUntilOpenPermitAvailable` to "AsyncGetter".
    Concurrently with this happening, the "AsyncGetter" tries to get from `ConcurrentPool`
    and is blocked because there are no more connections available. In such an execution,
    `waitUntilOpenPermitAvailable` cannot be completed by "AsyncGetter" because
    "AsyncGetter" is blocked doing a different task, which itself cannot be completed.
    
    a solution is to do `ConcurrentPool.get` and `waitUntilOpenPermitAvailable`
    in different threads. This way these two different kinds of tasks
    will not block each other by waiting in the same queue to be done by a single thread.
    
    Potential removal from `OpenConcurrencyLimiter.desiredConnectionSlots` when its empty.
    
    If `acquirePermitOrGetAvailableOpenedConnection` is called with `true` as `tryGetAvailable`,
    and `getPooledConnectionImmediately` throws an exception,
    then `expressDesireToGetAvailableConnection` is not called
    but `giveUpOnTryingToGetAvailableConnection` is still called, which is incorrect.
    
    JAVA-3928
    stIncMale committed Apr 25, 2021
    Configuration menu
    Copy the full SHA
    9e73482 View commit details
    Browse the repository at this point in the history
  3. Fix static checks

    JAVA-3927
    stIncMale committed Apr 25, 2021
    Configuration menu
    Copy the full SHA
    8d68448 View commit details
    Browse the repository at this point in the history

Commits on Apr 27, 2021

  1. Fix double release in DefaultConnectionPool/ConcurrentPool

    On one hand `ConcurrentPool.ensureMinSize` tries to not require the caller to do
    what can be done by the method itself, e.g., it releases the connection to the pool itself.
    On the other hand, always releasing in `ensureMinSize` may lead
    to releasing a permit for the same connection twice, thus not respecting the `maxSize`.
    
    It is not easy (requires an additional knob and logic)
    to prevent the caller (`DefaultConnectionPool`) from releasing a connection
    when initialization fails. It is, therefore, seems better to mandate that the caller
    releases a connection if initialization fails.
    
    JAVA-3927
    stIncMale committed Apr 27, 2021
    Configuration menu
    Copy the full SHA
    5ec1763 View commit details
    Browse the repository at this point in the history