I just reverted the added sync in PR #452, which violates the "no factory methods while holding locks" invariant. Strangely, the added tests for POOL-426 still pass. I think the race condition is still present and the general problem in POOL-413 remains unresolved.
On Thu, Dec 25, 2025 at 3:55 PM Phil Steitz <[email protected]> wrote: > The fix for POOL-425 included in the 2.13.0 release introduced a > regression that makes addObject no-op when maxIdle is set to a negative > value (no limit). The POOL-425 fix also failed to account for a race > condition reported in POOL-426. > > I have created a PR https://github.com/apache/commons-pool/pull/452 that > addresses > both issues. To avoid the race condition, I had to add synchronization to > addObject. I tried several ways to avoid the race by modifying create (as > suggested by Raju Gupta, the OP for POOL-426) but I could not find a way to > do that safely without introducing other issues. I don't see the added > sync in addObject as critical as this method is not used in hot code paths > internally and the lock that it acquires is the same lock that create will > subsequently acquire if it proceeds to add an object. > > The regression could be addressed in a simpler way by just fixing the > error in the code (failure to check for negative maxIdle). If there are > any doubts about the PR above, I am happy to make that simple change. In > any case, we should patch this quickly as it will likely break some apps > that use addObject with maxIdle unilimited. > > Thanks, all, and sorry for my mistake in the POOL-425 fix. > > Phil >
