You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "s-sanjay (via GitHub)" <gi...@apache.org> on 2024/09/14 18:08:53 UTC

[PR] This patch addresses a potential deadlock scenario in `GenericObjectPool` [commons-pool]

s-sanjay opened a new pull request, #346:
URL: https://github.com/apache/commons-pool/pull/346

   We ran into a issue in production when using commond-dbcp where a single thread will deadlock itself while connecting to mysql due to validation failure on create. We root caused this to a scenario where the thread will wait forever for object in the queue but no object is created. The code folow is as follows.
    
   In `GenericObjectPool`, inside the function `borrowObject`, If `borrowMaxWaitDuration` is negative and there is not a single created object (borrowed + idle), the caller would block indefinitely, especially in single-threaded scenarios, leading to a deadlock. This can happen because even though we try to create an object during borrow, create could return null if either `getMaxTotal()` returns 0 or creation fails because of various reasons ( eg: validateObject is false with `getTestOnCreate()` )
   
   - Added a check to throw a `NoSuchElementException` when there is not a single created (free + borrowed) object, preventing deadlocks.
   - Introduced a corresponding test case `testWhenNoObjectDoesNotBlock` in `TestGenericObjectPool` to validate the behavior.
   
   This ensures that the pool correctly fails to borrow objects when nothing is created, even with blocking enabled.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] This patch addresses a potential deadlock scenario in `GenericObjectPool` [commons-pool]

Posted by "s-sanjay (via GitHub)" <gi...@apache.org>.
s-sanjay commented on PR #346:
URL: https://github.com/apache/commons-pool/pull/346#issuecomment-2351301637

   @psteitz if there is only one caller how will the instance come back ? 
   The has to be object created for them to come back ? 
   If there are two thread and only one object in the pool then one thread can wait indefinitely till the other thread can return the object but with a single thread how will the thread ever get an object ? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] This patch addresses a potential deadlock scenario in `GenericObjectPool` [commons-pool]

Posted by "psteitz (via GitHub)" <gi...@apache.org>.
psteitz closed pull request #346: This patch addresses a potential deadlock scenario in `GenericObjectPool`
URL: https://github.com/apache/commons-pool/pull/346


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] This patch addresses a potential deadlock scenario in `GenericObjectPool` [commons-pool]

Posted by "psteitz (via GitHub)" <gi...@apache.org>.
psteitz commented on PR #346:
URL: https://github.com/apache/commons-pool/pull/346#issuecomment-2351179164

   @Gary Gregory ***@***.***> Good point.
   
   On Sat, Sep 14, 2024 at 2:27 PM Gary Gregory ***@***.***>
   wrote:
   
   > @psteitz <https://github.com/psteitz>
   > I agree. What is somewhat worrisome is that we don't have a test that
   > failed with what is obviously a regression.
   >
   > —
   > Reply to this email directly, view it on GitHub
   > <https://github.com/apache/commons-pool/pull/346#issuecomment-2351161701>,
   > or unsubscribe
   > <https://github.com/notifications/unsubscribe-auth/AALJJ2QD4OO6QDW4NWZP2DTZWSS5NAVCNFSM6AAAAABOHBOZKGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJRGE3DCNZQGE>
   > .
   > You are receiving this because you were mentioned.Message ID:
   > ***@***.***>
   >
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] This patch addresses a potential deadlock scenario in `GenericObjectPool` [commons-pool]

Posted by "psteitz (via GitHub)" <gi...@apache.org>.
psteitz commented on PR #346:
URL: https://github.com/apache/commons-pool/pull/346#issuecomment-2351143830

   I think that you are misunderstanding the API.  When you set maxWait to a negative value, you are telling the pool to block indefinitely until it can provide an instance.  It can provide an instance if there are idle objects in the pool or it has capacity and succeeds in creating a new instance.  The code you inserted will not allow the caller to wait after attempting the create for instances to come back to the pool.   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] This patch addresses a potential deadlock scenario in `GenericObjectPool` [commons-pool]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #346:
URL: https://github.com/apache/commons-pool/pull/346#issuecomment-2351161701

   @psteitz 
   I agree. What is somewhat worrisome is that we don't have a test that failed with what is obviously a regression.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org