You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "psteitz (via GitHub)" <gi...@apache.org> on 2025/12/25 22:42:54 UTC

[PR] Fix regression in fix for POOL-425, fix race in POOL-426. [commons-pool]

psteitz opened a new pull request, #452:
URL: https://github.com/apache/commons-pool/pull/452

   <!--
     Licensed to the Apache Software Foundation (ASF) under one
     or more contributor license agreements.  See the NOTICE file
     distributed with this work for additional information
     regarding copyright ownership.  The ASF licenses this file
     to you under the Apache License, Version 2.0 (the
     "License"); you may not use this file except in compliance
     with the License.  You may obtain a copy of the License at
   
       https://www.apache.org/licenses/LICENSE-2.0
   
     Unless required by applicable law or agreed to in writing,
     software distributed under the License is distributed on an
     "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
     KIND, either express or implied.  See the License for the
     specific language governing permissions and limitations
     under the License.
   -->
   
   Fixes the regression in the fix for POOL-425, which made addObject no-op when maxIdle is negative (no limit).  Also fixes the race condition identified in POOL-426.  


-- 
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] Fix regression in fix for POOL-425, fix race in POOL-426. [commons-pool]

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

   Yeah. That change is incorrect where we were taking the min value. I think the fix in this PR is correct for POOL-426. The fix is not correct for POOL-413.


-- 
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] Fix regression in fix for POOL-425, fix race in POOL-426. [commons-pool]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #452:
URL: https://github.com/apache/commons-pool/pull/452


-- 
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] Fix regression in fix for POOL-425, fix race in POOL-426. [commons-pool]

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

   When I add just the tests, they fail, as I expected they would.  I did not make changes to create, which it looks like one of the PRs against master was trying to do.  I don't think that change is correct.  The patch in this PR does not make changes to create.   


-- 
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] Fix regression in fix for POOL-425, fix race in POOL-426. [commons-pool]

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

   Addendum mentioned in previous comment for reference: https://github.com/apache/commons-pool/commit/8f46cde7835d29eaeae3b2c77488d9ccf5101f3c


-- 
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] Fix regression in fix for POOL-425, fix race in POOL-426. [commons-pool]

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

   @psteitz Could you also include the additional tests at https://github.com/apache/commons-pool/pull/451/changes as well ? There is an additional test for POOL-413 as well and I think this fixes POOL-413 as well. Thanks!


-- 
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] Fix regression in fix for POOL-425, fix race in POOL-426. [commons-pool]

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

   @psteitz I have added the details in POOL-413 to illustrate the issue. There is a PR for the failing test at #453. A fix has been proposed in the JIRA itself. Let me know your thoughts on this. 


-- 
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] Fix regression in fix for POOL-425, fix race in POOL-426. [commons-pool]

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

   This also fixes #450. Closing #450. 


-- 
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] Fix regression in fix for POOL-425, fix race in POOL-426. [commons-pool]

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

   I am really sorry, guys, but I am going to have to revert this change.  The added sync violates the "no factory methods while holding a lock" invariant, since it effectively extends the lock to all of create.  I will revert the change and implement the simple fix for the 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


Re: [PR] Fix regression in fix for POOL-425, fix race in POOL-426. [commons-pool]

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

   Closing [POOL-425](https://issues.apache.org/jira/browse/POOL-425) and [POOL-426](https://issues.apache.org/jira/browse/POOL-426).


-- 
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] Fix regression in fix for POOL-425, fix race in POOL-426. [commons-pool]

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

   Hello @rajucomp and @rajucomp 
   
   If I apply #451 to this PR locally (and edit to resolve the conflict), the tests pass and I get 2 checkstyle errors, so the test is OK. It might be cleaner for #451 to change and not use the same test method name as in this PR.
   
   I can bring both PRs in if you are both OK with the changes.
   


-- 
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] Fix regression in fix for POOL-425, fix race in POOL-426. [commons-pool]

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

   @garydgregory I think you can skip #451 as the current PR in the state at #452 contains the fix for POOL-426. 
   It would be better if we close #451 and try to merge #453 in a seperate PR. This would keep the history clean and short and easy to review. Let me know what you think. 


-- 
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] Fix regression in fix for POOL-425, fix race in POOL-426. [commons-pool]

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

   But thanks for the tests.  Please add a link or just paste them to POOL-413 JIRA as they do illustrate that issue.


-- 
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