Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Gary Gregory]

FTR, the build that failed ran on Ubuntu 22.04.2 LTS and Eclipse Adoptium
(temurin) 17.0.7+7.

Gary

On Thu, Jul 6, 2023, 17:42 Phil Steitz  wrote:

> 
>
> I guess it's good news that CI hit the error below when reviewing the PR
> that I had prepared for the POOL-391 fixes.  I only saw it once in many
> test runs and only on OpenJDK 20.0.1.  Looks like CI is running 17 on
> azure-linux.  I am pretty sure it has nothing to do with the changes in the
> PR, partly because I saw it once running the first RC code.
>
> This is very strange and troubling.  More eyeballs most welcome.  Here is
> the problem:
>
> The NPE below happens inside GKOP addIdleObject, which is called by
> addObject.  The code for addObject follows the standard pattern:
> register(key);
> try {
> addIdleObject(key, create(key));
> } finally {
> deregister(key);
> }
>
> So addIdleObject is called while the owning thread has the key
> "registered."  To register is key is basically sayinig you are interested
> in / about to do something to its associated pool.  There is a
> numInterested counter (attached to the keyed pool) that gets incremented
> when a thread registers a key and decrremented when the key is
> deregistered. Registration creates a pool and iniitializes its counter if
> there is no pool under the given key.   When pools are deregistered, the
> counter is checked and if the pool has no instances under management and
> the counter is zero, the pool is removed.  In this case, the registration
> in addObject should prevent the pool being removed before or during
> execution of addIdleObject, but the NPE means it has been removed.  Somehow
> numInterested is getting corrupted.
>
> I will keep trying to get this to happen and see if I can find scenarios
> where deregister is somehow called twice for one register.  Any suggestions
> or success getting this to happen with the code now in master most
> appreciated.
>
> Phil
>
> > 2. On MacOS 13.4.1, OpenJDK 20.0.1, I got the following test failure
> just one time and can't reproduce:
> >
> > java.util.concurrent.ExecutionException: java.lang.NullPointerException:
> Cannot invoke
> "org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getIdleObjects()"
> because the return value of "java.util.Map.get(Object)" is null
> >
> >   at
> java.base/java.util.concurrent.FutureTask.report(FutureTask.java:122)
> >   at
> java.base/java.util.concurrent.FutureTask.get(FutureTask.java:191)
> >   at
> org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testConcurrentBorrowAndClear$2(TestGenericKeyedObjectPool.java:1056)
> >   ... 71 more
> > Caused by: java.lang.NullPointerException: Cannot invoke
> "org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getIdleObjects()"
> because the return value of "java.util.Map.get(Object)" is null
> >   at
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.addIdleObject(GenericKeyedObjectPool.java:307)
> >   at
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.addObject(GenericKeyedObjectPool.java:332)
> >   at
> org.apache.commons.pool2.KeyedObjectPool.addObjects(KeyedObjectPool.java:136)
> >   at
> org.apache.commons.pool2.KeyedObjectPool.addObjects(KeyedObjectPool.java:113)
> >   at
> org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testConcurrentBorrowAndClear$0(TestGenericKeyedObjectPool.java:1036)
> >   at
> java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:577)
> >   at
> java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
> >   at
> java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
> >   at
> java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
> >   at java.base/java.lang.Thread.run(Thread.java:1623)
> >
> >
> >
> >
>

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Phil Steitz]

I think I may have figured this out.  I reopened and added a comment to
POOL-411.

Phil

On Thu, Jul 6, 2023 at 2:41 PM Phil Steitz  wrote:

> 
>
> I guess it's good news that CI hit the error below when reviewing the PR
> that I had prepared for the POOL-391 fixes.  I only saw it once in many
> test runs and only on OpenJDK 20.0.1.  Looks like CI is running 17 on
> azure-linux.  I am pretty sure it has nothing to do with the changes in the
> PR, partly because I saw it once running the first RC code.
>
> This is very strange and troubling.  More eyeballs most welcome.  Here is
> the problem:
>
> The NPE below happens inside GKOP addIdleObject, which is called by
> addObject.  The code for addObject follows the standard pattern:
> register(key);
> try {
> addIdleObject(key, create(key));
> } finally {
> deregister(key);
> }
>
> So addIdleObject is called while the owning thread has the key
> "registered."  To register is key is basically sayinig you are interested
> in / about to do something to its associated pool.  There is a
> numInterested counter (attached to the keyed pool) that gets incremented
> when a thread registers a key and decrremented when the key is
> deregistered. Registration creates a pool and iniitializes its counter if
> there is no pool under the given key.   When pools are deregistered, the
> counter is checked and if the pool has no instances under management and
> the counter is zero, the pool is removed.  In this case, the registration
> in addObject should prevent the pool being removed before or during
> execution of addIdleObject, but the NPE means it has been removed.  Somehow
> numInterested is getting corrupted.
>
> I will keep trying to get this to happen and see if I can find scenarios
> where deregister is somehow called twice for one register.  Any suggestions
> or success getting this to happen with the code now in master most
> appreciated.
>
> Phil
>
>> 2. On MacOS 13.4.1, OpenJDK 20.0.1, I got the following test failure just 
>> one time and can't reproduce:
>>
>> java.util.concurrent.ExecutionException: java.lang.NullPointerException: 
>> Cannot invoke 
>> "org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getIdleObjects()"
>>  because the return value of "java.util.Map.get(Object)" is null
>>
>>  at java.base/java.util.concurrent.FutureTask.report(FutureTask.java:122)
>>  at java.base/java.util.concurrent.FutureTask.get(FutureTask.java:191)
>>  at 
>> org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testConcurrentBorrowAndClear$2(TestGenericKeyedObjectPool.java:1056)
>>  ... 71 more
>> Caused by: java.lang.NullPointerException: Cannot invoke 
>> "org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getIdleObjects()"
>>  because the return value of "java.util.Map.get(Object)" is null
>>  at 
>> org.apache.commons.pool2.impl.GenericKeyedObjectPool.addIdleObject(GenericKeyedObjectPool.java:307)
>>  at 
>> org.apache.commons.pool2.impl.GenericKeyedObjectPool.addObject(GenericKeyedObjectPool.java:332)
>>  at 
>> org.apache.commons.pool2.KeyedObjectPool.addObjects(KeyedObjectPool.java:136)
>>  at 
>> org.apache.commons.pool2.KeyedObjectPool.addObjects(KeyedObjectPool.java:113)
>>  at 
>> org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testConcurrentBorrowAndClear$0(TestGenericKeyedObjectPool.java:1036)
>>  at 
>> java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:577)
>>  at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
>>  at 
>> java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
>>  at 
>> java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
>>  at java.base/java.lang.Thread.run(Thread.java:1623)
>>
>>
>>
>>

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Phil Steitz]

I guess it's good news that CI hit the error below when reviewing the PR
that I had prepared for the POOL-391 fixes.  I only saw it once in many
test runs and only on OpenJDK 20.0.1.  Looks like CI is running 17 on
azure-linux.  I am pretty sure it has nothing to do with the changes in the
PR, partly because I saw it once running the first RC code.

This is very strange and troubling.  More eyeballs most welcome.  Here is
the problem:

The NPE below happens inside GKOP addIdleObject, which is called by
addObject.  The code for addObject follows the standard pattern:
register(key);
try {
addIdleObject(key, create(key));
} finally {
deregister(key);
}

So addIdleObject is called while the owning thread has the key
"registered."  To register is key is basically sayinig you are interested
in / about to do something to its associated pool.  There is a
numInterested counter (attached to the keyed pool) that gets incremented
when a thread registers a key and decrremented when the key is
deregistered. Registration creates a pool and iniitializes its counter if
there is no pool under the given key.   When pools are deregistered, the
counter is checked and if the pool has no instances under management and
the counter is zero, the pool is removed.  In this case, the registration
in addObject should prevent the pool being removed before or during
execution of addIdleObject, but the NPE means it has been removed.  Somehow
numInterested is getting corrupted.

I will keep trying to get this to happen and see if I can find scenarios
where deregister is somehow called twice for one register.  Any suggestions
or success getting this to happen with the code now in master most
appreciated.

Phil

> 2. On MacOS 13.4.1, OpenJDK 20.0.1, I got the following test failure just one 
> time and can't reproduce:
>
> java.util.concurrent.ExecutionException: java.lang.NullPointerException: 
> Cannot invoke 
> "org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getIdleObjects()"
>  because the return value of "java.util.Map.get(Object)" is null
>
>   at java.base/java.util.concurrent.FutureTask.report(FutureTask.java:122)
>   at java.base/java.util.concurrent.FutureTask.get(FutureTask.java:191)
>   at 
> org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testConcurrentBorrowAndClear$2(TestGenericKeyedObjectPool.java:1056)
>   ... 71 more
> Caused by: java.lang.NullPointerException: Cannot invoke 
> "org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getIdleObjects()"
>  because the return value of "java.util.Map.get(Object)" is null
>   at 
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.addIdleObject(GenericKeyedObjectPool.java:307)
>   at 
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.addObject(GenericKeyedObjectPool.java:332)
>   at 
> org.apache.commons.pool2.KeyedObjectPool.addObjects(KeyedObjectPool.java:136)
>   at 
> org.apache.commons.pool2.KeyedObjectPool.addObjects(KeyedObjectPool.java:113)
>   at 
> org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testConcurrentBorrowAndClear$0(TestGenericKeyedObjectPool.java:1036)
>   at 
> java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:577)
>   at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
>   at 
> java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
>   at 
> java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
>   at java.base/java.lang.Thread.run(Thread.java:1623)
>
>
>
>

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Gary Gregory]

Hi Phil,

Starting a new mailing thread is a good idea at this point. I'll just
mention: I really do not want to roll back code changes in master. If you
want to pick a commit in the past and create a branch 2.x from that, it's
fine with me, or tell me, and I'll be happy to create the branch. Then I
can switch master to 3.0.

Gary


On Mon, Jul 3, 2023, 11:31 Phil Steitz  wrote:

> On Mon, Jul 3, 2023 at 6:41 AM Gary Gregory 
> wrote:
>
> > On Thu, Jun 29, 2023 at 5:08 PM Phil Steitz 
> wrote:
> > >
> > > On Thu, Jun 29, 2023 at 11:39 AM Gary Gregory 
> > > wrote:
> > >
> > > > Great presentation in the video Elliotte. Thanks for sharing the
> link.
> > > >
> > >
> > > +1 many thanks.
> > >
> > > Now back to our hero.  Let me pretend to be one of the people in the
> > > audience of the video.
> > >
> > > We have this library that is used by all kinds of "programs" [1] and we
> > are
> > > not sure how best to a) type our own exceptions (the ones we generate)
> > and
> > > b) propagate the ones generated by user-supplied object factories.
> > Broadly
> > > speaking, we have four kinds of exceptions to deal with:
> > >
> > > 0) User-supplied object factories that the pool uses to perform pooled
> > > object lifecycle operations may throw checked or unchecked exceptions.
> > > Many of these will be the classic variety you mention in the video -
> > > database is dead, etc when the factory is trying to open a physical
> > > connection.  The smelly "throws Exception" in place now lets us just
> pass
> > > these all up the stack, but essentially untyped.
> > > 1) Pool clients want to do something, but the pool can't do it without
> > > violating one of its invariants (most common is a client wants to
> borrow
> > an
> > > object and, after waiting the configured interval, there is no capacity
> > to
> > > serve).  We are inconsistent here.  In the out of capacity case, we
> throw
> > > NoSuchElementException (probably bogus according to your taxonomy), but
> > in
> > > some cases - e.g. addObject - we silently no-op.
> > > 2) An API is abused (e.g. return an object that did not come from the
> > pool
> > > or try to start borrowing objects before providing a factory)
> > > 3) Something happens that should not be possible (indicating some kind
> of
> > > concurrency bug in [pool] itself or unanticipated craziness from a
> > factory
> > > or client code)
> > >
> > > There is a lot of complexity in the 0)-1) cases because some failures
> are
> > > more important to both the pool and clients than others (for example,
> > > factory exceptions thrown in object validation or destruction are
> > different
> > > from those thrown in object creation).
> > >
> > > If I understand your advice, it would be:
> > > 0) checked if that is what comes from the factory; just propagate
> > otherwise
> > > 1) checked?  Maybe actually no exception - handle via return values or
> > > somesuch?  By far the most common here is NoSuchElementException.  I
> > guess
> > > it's OK to see pool exhaustion as "environmental" [2], so probably
> > checked
> > > is actually right
> > > 2) unchecked
> > > 3) unchecked
> > >
> > > Assuming I got 0) right at least, the follow-up is how do we get the
> > right
> > > exceptions passed up the stack?  What Gary is proposing is adding a
> type
> > > parameter at the pool level.  Could be that is the best solution, but
> > that
> > > adds to complexity of the API and I keep wanting it to be defined at
> the
> > > factory level somehow and ideally have it default to something so users
> > who
> > > don't want to think about it (maybe because their factories only throw
> > > unchecked exceptions) don't have to think about it.
> > >
> > > I would be remiss not to add a closing (at this point probably
> annoying)
> > > comment to the effect that this should be a 3.0 discussion :)
> >
> > I am OK with all of this being for 3.0, which can be as soon as we want.
> >
>
> Thanks, Gary.  We should probably end this thread then and start a new one
> on 3.0 planning.  As a side note, I discovered that the failures reported
> to the console but not causing test failure that I saw were real, due to
> problems in GKOP reuseCapacity.  The OP test case for POOL-391 is
> demanding, but we should be able to handle it. It was not causing the test
> to fail because the runner does not see failures in spawned threads.  My
> bad not knowing that.  Once I set it up so the failures would fail the
> test, they did consistently.  I have been working on and off on this over
> the last week and I think I am close to a fix.  Once I have this, I think
> it would be good to roll a 2.12 without the compat break because there are
> quite a few bug fixes in the RC.  Would you be OK either rolling back the
> added type parameter change or moving it to a new 3.0 branch?  Or I guess
> alternatively, creating a 2.x branch without that change?   I can help with
> either of these.
>
> Phil
>
> >
> > Gary
> >
> > >
> > > Phil
> > >
> > > [1] It 

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Phil Steitz]

On Mon, Jul 3, 2023 at 6:41 AM Gary Gregory  wrote:

> On Thu, Jun 29, 2023 at 5:08 PM Phil Steitz  wrote:
> >
> > On Thu, Jun 29, 2023 at 11:39 AM Gary Gregory 
> > wrote:
> >
> > > Great presentation in the video Elliotte. Thanks for sharing the link.
> > >
> >
> > +1 many thanks.
> >
> > Now back to our hero.  Let me pretend to be one of the people in the
> > audience of the video.
> >
> > We have this library that is used by all kinds of "programs" [1] and we
> are
> > not sure how best to a) type our own exceptions (the ones we generate)
> and
> > b) propagate the ones generated by user-supplied object factories.
> Broadly
> > speaking, we have four kinds of exceptions to deal with:
> >
> > 0) User-supplied object factories that the pool uses to perform pooled
> > object lifecycle operations may throw checked or unchecked exceptions.
> > Many of these will be the classic variety you mention in the video -
> > database is dead, etc when the factory is trying to open a physical
> > connection.  The smelly "throws Exception" in place now lets us just pass
> > these all up the stack, but essentially untyped.
> > 1) Pool clients want to do something, but the pool can't do it without
> > violating one of its invariants (most common is a client wants to borrow
> an
> > object and, after waiting the configured interval, there is no capacity
> to
> > serve).  We are inconsistent here.  In the out of capacity case, we throw
> > NoSuchElementException (probably bogus according to your taxonomy), but
> in
> > some cases - e.g. addObject - we silently no-op.
> > 2) An API is abused (e.g. return an object that did not come from the
> pool
> > or try to start borrowing objects before providing a factory)
> > 3) Something happens that should not be possible (indicating some kind of
> > concurrency bug in [pool] itself or unanticipated craziness from a
> factory
> > or client code)
> >
> > There is a lot of complexity in the 0)-1) cases because some failures are
> > more important to both the pool and clients than others (for example,
> > factory exceptions thrown in object validation or destruction are
> different
> > from those thrown in object creation).
> >
> > If I understand your advice, it would be:
> > 0) checked if that is what comes from the factory; just propagate
> otherwise
> > 1) checked?  Maybe actually no exception - handle via return values or
> > somesuch?  By far the most common here is NoSuchElementException.  I
> guess
> > it's OK to see pool exhaustion as "environmental" [2], so probably
> checked
> > is actually right
> > 2) unchecked
> > 3) unchecked
> >
> > Assuming I got 0) right at least, the follow-up is how do we get the
> right
> > exceptions passed up the stack?  What Gary is proposing is adding a type
> > parameter at the pool level.  Could be that is the best solution, but
> that
> > adds to complexity of the API and I keep wanting it to be defined at the
> > factory level somehow and ideally have it default to something so users
> who
> > don't want to think about it (maybe because their factories only throw
> > unchecked exceptions) don't have to think about it.
> >
> > I would be remiss not to add a closing (at this point probably annoying)
> > comment to the effect that this should be a 3.0 discussion :)
>
> I am OK with all of this being for 3.0, which can be as soon as we want.
>

Thanks, Gary.  We should probably end this thread then and start a new one
on 3.0 planning.  As a side note, I discovered that the failures reported
to the console but not causing test failure that I saw were real, due to
problems in GKOP reuseCapacity.  The OP test case for POOL-391 is
demanding, but we should be able to handle it. It was not causing the test
to fail because the runner does not see failures in spawned threads.  My
bad not knowing that.  Once I set it up so the failures would fail the
test, they did consistently.  I have been working on and off on this over
the last week and I think I am close to a fix.  Once I have this, I think
it would be good to roll a 2.12 without the compat break because there are
quite a few bug fixes in the RC.  Would you be OK either rolling back the
added type parameter change or moving it to a new 3.0 branch?  Or I guess
alternatively, creating a 2.x branch without that change?   I can help with
either of these.

Phil

>
> Gary
>
> >
> > Phil
> >
> > [1] It is hard for us to understand what is and is not in "our program"
> as
> > library developers.  Our code is always running inside someone else's
> > program, often using yet another third party's code.  So for example, in
> a
> > common use case, [pool] is used by [dbcp] which is used by tomcat, with
> > factories specified in a user webapp running in tomcat, wrapped into
> [dbcp]
> > and managed by [pool].
> > [2] Though many/most actual cases of this in production code end up being
> > the result of self-DOS due to programming errors.
> >
> > >
> > > Gary
> > >
> > > On Thu, Jun 29, 2023, 10:33 Elliotte 

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Gary Gregory]

On Thu, Jun 29, 2023 at 5:08 PM Phil Steitz  wrote:
>
> On Thu, Jun 29, 2023 at 11:39 AM Gary Gregory 
> wrote:
>
> > Great presentation in the video Elliotte. Thanks for sharing the link.
> >
>
> +1 many thanks.
>
> Now back to our hero.  Let me pretend to be one of the people in the
> audience of the video.
>
> We have this library that is used by all kinds of "programs" [1] and we are
> not sure how best to a) type our own exceptions (the ones we generate) and
> b) propagate the ones generated by user-supplied object factories.  Broadly
> speaking, we have four kinds of exceptions to deal with:
>
> 0) User-supplied object factories that the pool uses to perform pooled
> object lifecycle operations may throw checked or unchecked exceptions.
> Many of these will be the classic variety you mention in the video -
> database is dead, etc when the factory is trying to open a physical
> connection.  The smelly "throws Exception" in place now lets us just pass
> these all up the stack, but essentially untyped.
> 1) Pool clients want to do something, but the pool can't do it without
> violating one of its invariants (most common is a client wants to borrow an
> object and, after waiting the configured interval, there is no capacity to
> serve).  We are inconsistent here.  In the out of capacity case, we throw
> NoSuchElementException (probably bogus according to your taxonomy), but in
> some cases - e.g. addObject - we silently no-op.
> 2) An API is abused (e.g. return an object that did not come from the pool
> or try to start borrowing objects before providing a factory)
> 3) Something happens that should not be possible (indicating some kind of
> concurrency bug in [pool] itself or unanticipated craziness from a factory
> or client code)
>
> There is a lot of complexity in the 0)-1) cases because some failures are
> more important to both the pool and clients than others (for example,
> factory exceptions thrown in object validation or destruction are different
> from those thrown in object creation).
>
> If I understand your advice, it would be:
> 0) checked if that is what comes from the factory; just propagate otherwise
> 1) checked?  Maybe actually no exception - handle via return values or
> somesuch?  By far the most common here is NoSuchElementException.  I guess
> it's OK to see pool exhaustion as "environmental" [2], so probably checked
> is actually right
> 2) unchecked
> 3) unchecked
>
> Assuming I got 0) right at least, the follow-up is how do we get the right
> exceptions passed up the stack?  What Gary is proposing is adding a type
> parameter at the pool level.  Could be that is the best solution, but that
> adds to complexity of the API and I keep wanting it to be defined at the
> factory level somehow and ideally have it default to something so users who
> don't want to think about it (maybe because their factories only throw
> unchecked exceptions) don't have to think about it.
>
> I would be remiss not to add a closing (at this point probably annoying)
> comment to the effect that this should be a 3.0 discussion :)

I am OK with all of this being for 3.0, which can be as soon as we want.

Gary

>
> Phil
>
> [1] It is hard for us to understand what is and is not in "our program" as
> library developers.  Our code is always running inside someone else's
> program, often using yet another third party's code.  So for example, in a
> common use case, [pool] is used by [dbcp] which is used by tomcat, with
> factories specified in a user webapp running in tomcat, wrapped into [dbcp]
> and managed by [pool].
> [2] Though many/most actual cases of this in production code end up being
> the result of self-DOS due to programming errors.
>
> >
> > Gary
> >
> > On Thu, Jun 29, 2023, 10:33 Elliotte Rusty Harold 
> > wrote:
> >
> > > On Thu, Jun 29, 2023 at 10:10 AM Gilles Sadowski 
> > > wrote:
> > > >
> > > > Le jeu. 29 juin 2023 à 15:22, Elliotte Rusty Harold
> > > >  a écrit :
> > > > >
> > > > > On Thu, Jun 29, 2023 at 9:07 AM Gilles Sadowski <
> > gillese...@gmail.com>
> > > wrote:
> > > > > >
> > > > > > Hello.
> > > > > >
> > > > > > Le jeu. 29 juin 2023 à 14:44, Gary Gregory  > >
> > > a écrit :
> > > > >
> > > > > > I agree with the second part assuming the *current* Java
> > > > > > best practices, not because of old APIs that are here to stay
> > > > > > only because of infinite backwards compatibility, and not
> > > > > > because they are deemed perfect.
> > > > >
> > > > >
> > > > > Best practices on this haven't changed since Java 1.0 (Possibly Java
> > > > > 1.0 alpha 3 if I recall versions correctly).
> > > > >
> > > > > Checked exceptions: all errors arising from external input to the
> > > program.
> > > > > Runtime exceptions: program bugs
> > > >
> > > > A pile of arguments (tally is largely against *any* use of checked
> > > exceptions):
> > > >
> > >
> > https://ohadshai.medium.com/its-almost-2020-and-yet-checked-exceptions-are-still-a-thing-bfaa24c8997e
> > >
> > > tldr; he's 

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Phil Steitz]

On Thu, Jun 29, 2023 at 11:39 AM Gary Gregory 
wrote:

> Great presentation in the video Elliotte. Thanks for sharing the link.
>

+1 many thanks.

Now back to our hero.  Let me pretend to be one of the people in the
audience of the video.

We have this library that is used by all kinds of "programs" [1] and we are
not sure how best to a) type our own exceptions (the ones we generate) and
b) propagate the ones generated by user-supplied object factories.  Broadly
speaking, we have four kinds of exceptions to deal with:

0) User-supplied object factories that the pool uses to perform pooled
object lifecycle operations may throw checked or unchecked exceptions.
Many of these will be the classic variety you mention in the video -
database is dead, etc when the factory is trying to open a physical
connection.  The smelly "throws Exception" in place now lets us just pass
these all up the stack, but essentially untyped.
1) Pool clients want to do something, but the pool can't do it without
violating one of its invariants (most common is a client wants to borrow an
object and, after waiting the configured interval, there is no capacity to
serve).  We are inconsistent here.  In the out of capacity case, we throw
NoSuchElementException (probably bogus according to your taxonomy), but in
some cases - e.g. addObject - we silently no-op.
2) An API is abused (e.g. return an object that did not come from the pool
or try to start borrowing objects before providing a factory)
3) Something happens that should not be possible (indicating some kind of
concurrency bug in [pool] itself or unanticipated craziness from a factory
or client code)

There is a lot of complexity in the 0)-1) cases because some failures are
more important to both the pool and clients than others (for example,
factory exceptions thrown in object validation or destruction are different
from those thrown in object creation).

If I understand your advice, it would be:
0) checked if that is what comes from the factory; just propagate otherwise
1) checked?  Maybe actually no exception - handle via return values or
somesuch?  By far the most common here is NoSuchElementException.  I guess
it's OK to see pool exhaustion as "environmental" [2], so probably checked
is actually right
2) unchecked
3) unchecked

Assuming I got 0) right at least, the follow-up is how do we get the right
exceptions passed up the stack?  What Gary is proposing is adding a type
parameter at the pool level.  Could be that is the best solution, but that
adds to complexity of the API and I keep wanting it to be defined at the
factory level somehow and ideally have it default to something so users who
don't want to think about it (maybe because their factories only throw
unchecked exceptions) don't have to think about it.

I would be remiss not to add a closing (at this point probably annoying)
comment to the effect that this should be a 3.0 discussion :)

Phil

[1] It is hard for us to understand what is and is not in "our program" as
library developers.  Our code is always running inside someone else's
program, often using yet another third party's code.  So for example, in a
common use case, [pool] is used by [dbcp] which is used by tomcat, with
factories specified in a user webapp running in tomcat, wrapped into [dbcp]
and managed by [pool].
[2] Though many/most actual cases of this in production code end up being
the result of self-DOS due to programming errors.

>
> Gary
>
> On Thu, Jun 29, 2023, 10:33 Elliotte Rusty Harold 
> wrote:
>
> > On Thu, Jun 29, 2023 at 10:10 AM Gilles Sadowski 
> > wrote:
> > >
> > > Le jeu. 29 juin 2023 à 15:22, Elliotte Rusty Harold
> > >  a écrit :
> > > >
> > > > On Thu, Jun 29, 2023 at 9:07 AM Gilles Sadowski <
> gillese...@gmail.com>
> > wrote:
> > > > >
> > > > > Hello.
> > > > >
> > > > > Le jeu. 29 juin 2023 à 14:44, Gary Gregory  >
> > a écrit :
> > > >
> > > > > I agree with the second part assuming the *current* Java
> > > > > best practices, not because of old APIs that are here to stay
> > > > > only because of infinite backwards compatibility, and not
> > > > > because they are deemed perfect.
> > > >
> > > >
> > > > Best practices on this haven't changed since Java 1.0 (Possibly Java
> > > > 1.0 alpha 3 if I recall versions correctly).
> > > >
> > > > Checked exceptions: all errors arising from external input to the
> > program.
> > > > Runtime exceptions: program bugs
> > >
> > > A pile of arguments (tally is largely against *any* use of checked
> > exceptions):
> > >
> >
> https://ohadshai.medium.com/its-almost-2020-and-yet-checked-exceptions-are-still-a-thing-bfaa24c8997e
> >
> > tldr; he's wrong. More details:
> > https://www.youtube.com/watch?v=rJ-Ihh7RNao=419s
> >
> > > >
> > > > I don't know which applies here, but 99% of the time this is all you
> > > > need to consider to figure out whether a checked or runtime exception
> > > > is called for. There are indeed a lot of old broken APIs that don't
> > > > follow these simple rules,

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Gary Gregory]

Great presentation in the video Elliotte. Thanks for sharing the link.

Gary

On Thu, Jun 29, 2023, 10:33 Elliotte Rusty Harold 
wrote:

> On Thu, Jun 29, 2023 at 10:10 AM Gilles Sadowski 
> wrote:
> >
> > Le jeu. 29 juin 2023 à 15:22, Elliotte Rusty Harold
> >  a écrit :
> > >
> > > On Thu, Jun 29, 2023 at 9:07 AM Gilles Sadowski 
> wrote:
> > > >
> > > > Hello.
> > > >
> > > > Le jeu. 29 juin 2023 à 14:44, Gary Gregory 
> a écrit :
> > >
> > > > I agree with the second part assuming the *current* Java
> > > > best practices, not because of old APIs that are here to stay
> > > > only because of infinite backwards compatibility, and not
> > > > because they are deemed perfect.
> > >
> > >
> > > Best practices on this haven't changed since Java 1.0 (Possibly Java
> > > 1.0 alpha 3 if I recall versions correctly).
> > >
> > > Checked exceptions: all errors arising from external input to the
> program.
> > > Runtime exceptions: program bugs
> >
> > A pile of arguments (tally is largely against *any* use of checked
> exceptions):
> >
> https://ohadshai.medium.com/its-almost-2020-and-yet-checked-exceptions-are-still-a-thing-bfaa24c8997e
>
> tldr; he's wrong. More details:
> https://www.youtube.com/watch?v=rJ-Ihh7RNao=419s
>
> > >
> > > I don't know which applies here, but 99% of the time this is all you
> > > need to consider to figure out whether a checked or runtime exception
> > > is called for. There are indeed a lot of old broken APIs that don't
> > > follow these simple rules,
> >
> > The rules which you stated are far from simple because "external"
> > depends on the POV (more below).
> >
> > Commons components like [RNG], [Geometry], [Numbers], [Math]
> > only throw unchecked exceptions.
>
> Libraries you link to are not external to the program. Runtime
> exceptions might be appropriate here.
>
> > The only rule that ever made sense to me is from J. Bloch's "Effective
> > Java":
> > ---CUT---
> > Use checked exceptions for recoverable conditions and runtime
> > exceptions for programming errors
> > ---CUT---
>
> This is about the only thing in that book where I disagree with Bloch.
> At the very least this is not said as well as it needs to be.
> Recoverable vs unrecoverable distinguishes Errors from Exceptions, not
> runtime exceptions from checked exceptions. You can interpret that
> sentence to mean "bugs are not recoverable short of fixing the code"
> in which case sure, runtime exceptions are not recoverable.
>
> > For example, *all* precondition failures are, from the POV of the
> > called function, a programming error (akin to NPE or IAE).
>
> This is correct. NPEs and IAEs are programming errors.
>
> > Whether the root cause is an "internal" or "external" error is irrelevant
> > at the point where the issue is detected (where the runtime exception
> > just conveys that "the call was inappropriate").
>
> It's not about the root cause. It's about the immediate cause. Passing
> null or another illegal argument to a method that isn't ready to
> receive it is a programming error, i.e. a bug, and therefore these are
> properly runtime exceptions.
>
> > > and it's never fun when a system goes down
> > > in production because some random JSON yahoo thought checked
> > > exceptions were "icky".
> >
> > A bug of the application, by the "absent-minded" developer mentioned
> > in the previous mail.
> > In Java, nothing prevents the throwing of a runtime
> > exception; the developer *always* must enclose "foreign" code in a
> > "try/catch" block, to protect the system.  There is no need for checked
> > exceptions just to remember this (really) simple rule.
>
> Had the JSON library properly used checked exceptions, the programmer
> would not have been able to compile code that contained this mistake.
> The reason we have strong static typing, unit tests, and checked
> exceptions is that programmers are imperfect humans. Let the machines
> do the work that machines are good at, including checking whether
> errors are properly handled.
>
> > > Lately I've been working in Python, and error handling practice in
> > > this community is abominable. The lack of checked exceptions is a
> > > large reason why Python code is expected to be unreliable and Python
> > > programs routinely dump stack.
> >
> > I've heard that it's rather the result of "script"-oriented design, or
> lack
> > thereof... :-)
>
> There's more than one reason. :-)
>
>
> --
> Elliotte Rusty Harold
> elh...@ibiblio.org
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Elliotte Rusty Harold]

On Thu, Jun 29, 2023 at 11:43 AM Gilles Sadowski  wrote:

> I think I can understand your rationale: Something that can be
> corrected at runtime should be signalled by a checked exception.
> Right?

No, not at all. Recoverability is **not** the distinction between
checked and unchecked exceptions.

Checked exceptions should arise from conditions outside the program
itself, like a malformed file or a broken network connection.
Runtime exceptions should arise from issues inside the program, like
invoking a method on a variable whose value is null, or passing -28 to
a method that will only accept positive numbers.

> However I don't understand why you now add "Error" to the mix...

Because recoverability is the distinction between Exceptions and
Errors. Most of the time exceptions, checked and unchecked, can be
recovered from. Errors usually can't be.


-- 
Elliotte Rusty Harold
elh...@ibiblio.org

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Gilles Sadowski]

Le jeu. 29 juin 2023 à 17:42, Elliotte Rusty Harold
 a écrit :
>
> On Thu, Jun 29, 2023 at 10:48 AM Gilles Sadowski  wrote:
>
> > The situation is recoverable from the caller's POV, by performing a
> > *new* call, with a correct argument.  We certainly don't need a special
> > kind of exception[1] to handle this situation.  What's wrong with catching
> > a "RuntimeException" if you know how to try again?
> >
>
> The problem isn't with catching a RuntimeException. The problem is
> with *not* catching the RuntimeException because the compiler doesn't
> make you do this, and the programmer forgets.

Yes, but I handled that case from the outset:  If the programmer forgets,
that is a bug!  And this kind of bug has no particular status wrt any other
bug (that would throw a "RuntimeException" and make the program crash
anyways if the "try/catch" block is missing).
If "no crash" is an application requirement, it probably should be ensured
through appropriate coverage (unit/fuzzy testing).

>
> Checked exceptions are for cases that SHOULD have an error handler,
> and simply makes the omission of one an easy-to-fix compile time error
> instead of a hard-to-fix runtime error.

In most cases, the sense that lower-level code is able to tell "you might
want to handle this" leads to every function advertising "Exception", just
in case some new "advice" might be deemed useful in the future; then
the cure to calling such a painful library is
---CUT---
try {
callPainfulLibrary(someArgs);
} catch(Exception e) {
thrown new RuntimeException(e);
}
---CUT---

>
> If the code in question shouldn't have an error handler, typically
> because the code in question is buggy and should be fixed so that the
> exception is never thrown in the first place, then a runtime exception
> is appropriate.
>

That's the part on which almost all agree except the writers of "painful"
libraries.  ;-)

Best regards,
Gilles

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Gilles Sadowski]

Le jeu. 29 juin 2023 à 16:33, Elliotte Rusty Harold
 a écrit :
>
> On Thu, Jun 29, 2023 at 10:10 AM Gilles Sadowski  wrote:
> >
> > Le jeu. 29 juin 2023 à 15:22, Elliotte Rusty Harold
> >  a écrit :
> > >
> > > On Thu, Jun 29, 2023 at 9:07 AM Gilles Sadowski  
> > > wrote:
> > > >
> > > > Hello.
> > > >
> > > > Le jeu. 29 juin 2023 à 14:44, Gary Gregory  a 
> > > > écrit :
> > >
> > > > I agree with the second part assuming the *current* Java
> > > > best practices, not because of old APIs that are here to stay
> > > > only because of infinite backwards compatibility, and not
> > > > because they are deemed perfect.
> > >
> > >
> > > Best practices on this haven't changed since Java 1.0 (Possibly Java
> > > 1.0 alpha 3 if I recall versions correctly).
> > >
> > > Checked exceptions: all errors arising from external input to the program.
> > > Runtime exceptions: program bugs
> >
> > A pile of arguments (tally is largely against *any* use of checked 
> > exceptions):
> >   
> > https://ohadshai.medium.com/its-almost-2020-and-yet-checked-exceptions-are-still-a-thing-bfaa24c8997e
>
> tldr; he's wrong. More details:
> https://www.youtube.com/watch?v=rJ-Ihh7RNao=419s

At that point, and more than 10 minutes on, you explain the advantages
of exception vs no exception.  There is no debate: "exception" is much
better than error-code checking.  It was already in C++.

> > >
> > > I don't know which applies here, but 99% of the time this is all you
> > > need to consider to figure out whether a checked or runtime exception
> > > is called for. There are indeed a lot of old broken APIs that don't
> > > follow these simple rules,
> >
> > The rules which you stated are far from simple because "external"
> > depends on the POV (more below).
> >
> > Commons components like [RNG], [Geometry], [Numbers], [Math]
> > only throw unchecked exceptions.
>
> Libraries you link to are not external to the program. Runtime
> exceptions might be appropriate here.
>
> > The only rule that ever made sense to me is from J. Bloch's "Effective
> > Java":
> > ---CUT---
> > Use checked exceptions for recoverable conditions and runtime
> > exceptions for programming errors
> > ---CUT---
>
> This is about the only thing in that book where I disagree with Bloch.
> At the very least this is not said as well as it needs to be.
> Recoverable vs unrecoverable distinguishes Errors from Exceptions, not
> runtime exceptions from checked exceptions. You can interpret that
> sentence to mean "bugs are not recoverable short of fixing the code"
> in which case sure, runtime exceptions are not recoverable.

I think I can understand your rationale: Something that can be
corrected at runtime should be signalled by a checked exception.
Right?

However I don't understand why you now add "Error" to the mix...

And that's possibly the main issue:  There is no convincing use-case
for so many "Throwable" categories at the JDK level:  What is
recoverable, or not, is ultimately a domain-related convention, and
also perhaps a matter of definition in the context of a particular design.
And the unchecked exception is simply the most convenient because
it provides the significant advantage (over error code checking) while
not forcing anyone to litter the code with mandatory "throws" clause,
even if the code does not intend to handle any of the advertised
exceptions.

>
> > For example, *all* precondition failures are, from the POV of the
> > called function, a programming error (akin to NPE or IAE).
>
> This is correct. NPEs and IAEs are programming errors.
>
> > Whether the root cause is an "internal" or "external" error is irrelevant
> > at the point where the issue is detected (where the runtime exception
> > just conveys that "the call was inappropriate").
>
> It's not about the root cause. It's about the immediate cause. Passing
> null or another illegal argument to a method that isn't ready to
> receive it is a programming error, i.e. a bug, and therefore these are
> properly runtime exceptions.

It's not necessarily a "bug" in the sense of "programming error".  For
example, a program reads user input with some rule on whether to
accept it.  If not accepted it throws.  Whether it's recoverable depends
on the wanted functionality (e.g. allow multiple attempts or not).

> > > and it's never fun when a system goes down
> > > in production because some random JSON yahoo thought checked
> > > exceptions were "icky".
> >
> > A bug of the application, by the "absent-minded" developer mentioned
> > in the previous mail.
> > In Java, nothing prevents the throwing of a runtime
> > exception; the developer *always* must enclose "foreign" code in a
> > "try/catch" block, to protect the system.  There is no need for checked
> > exceptions just to remember this (really) simple rule.
>
> Had the JSON library properly used checked exceptions, the programmer
> would not have been able to compile code that contained this mistake.
> The reason we have strong static typing, unit 

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Elliotte Rusty Harold]

On Thu, Jun 29, 2023 at 10:48 AM Gilles Sadowski  wrote:

> The situation is recoverable from the caller's POV, by performing a
> *new* call, with a correct argument.  We certainly don't need a special
> kind of exception[1] to handle this situation.  What's wrong with catching
> a "RuntimeException" if you know how to try again?
>

The problem isn't with catching a RuntimeException. The problem is
with *not* catching the RuntimeException because the compiler doesn't
make you do this, and the programmer forgets.

Checked exceptions are for cases that SHOULD have an error handler,
and simply makes the omission of one an easy-to-fix compile time error
instead of a hard-to-fix runtime error.

If the code in question shouldn't have an error handler, typically
because the code in question is buggy and should be fixed so that the
exception is never thrown in the first place, then a runtime exception
is appropriate.

-- 
Elliotte Rusty Harold
elh...@ibiblio.org

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Gilles Sadowski]

Le jeu. 29 juin 2023 à 16:18, Gary Gregory  a écrit :
>
> > I never could find a convincing example of "recoverable conditions".
>
> Really? How about typing in a bad server name, user name, or password?

Unfortunately we use different definitions of "recoverable".

IMO, they are typically *non*-recoverable errors: Input is plain wrong
and the called code can do *nothing* about it (apart from telling the
caller to fix it on its end, and try again).

> That's a classic recoverable exception (in JDBC land for example).

The situation is recoverable from the caller's POV, by performing a
*new* call, with a correct argument.  We certainly don't need a special
kind of exception[1] to handle this situation.  What's wrong with catching
a "RuntimeException" if you know how to try again?

JDBC is old.  Does any new, similarly widely used API, make similar
use of checked exceptions?

Regards,
Gilles

[1] In recent years Java borrowed several good ideas from other
languages; AFAIK, no language borrowed the "checked exception"
idea from Java.

>> [...]

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Elliotte Rusty Harold]

On Thu, Jun 29, 2023 at 10:10 AM Gilles Sadowski  wrote:
>
> Le jeu. 29 juin 2023 à 15:22, Elliotte Rusty Harold
>  a écrit :
> >
> > On Thu, Jun 29, 2023 at 9:07 AM Gilles Sadowski  
> > wrote:
> > >
> > > Hello.
> > >
> > > Le jeu. 29 juin 2023 à 14:44, Gary Gregory  a 
> > > écrit :
> >
> > > I agree with the second part assuming the *current* Java
> > > best practices, not because of old APIs that are here to stay
> > > only because of infinite backwards compatibility, and not
> > > because they are deemed perfect.
> >
> >
> > Best practices on this haven't changed since Java 1.0 (Possibly Java
> > 1.0 alpha 3 if I recall versions correctly).
> >
> > Checked exceptions: all errors arising from external input to the program.
> > Runtime exceptions: program bugs
>
> A pile of arguments (tally is largely against *any* use of checked 
> exceptions):
>   
> https://ohadshai.medium.com/its-almost-2020-and-yet-checked-exceptions-are-still-a-thing-bfaa24c8997e

tldr; he's wrong. More details:
https://www.youtube.com/watch?v=rJ-Ihh7RNao=419s

> >
> > I don't know which applies here, but 99% of the time this is all you
> > need to consider to figure out whether a checked or runtime exception
> > is called for. There are indeed a lot of old broken APIs that don't
> > follow these simple rules,
>
> The rules which you stated are far from simple because "external"
> depends on the POV (more below).
>
> Commons components like [RNG], [Geometry], [Numbers], [Math]
> only throw unchecked exceptions.

Libraries you link to are not external to the program. Runtime
exceptions might be appropriate here.

> The only rule that ever made sense to me is from J. Bloch's "Effective
> Java":
> ---CUT---
> Use checked exceptions for recoverable conditions and runtime
> exceptions for programming errors
> ---CUT---

This is about the only thing in that book where I disagree with Bloch.
At the very least this is not said as well as it needs to be.
Recoverable vs unrecoverable distinguishes Errors from Exceptions, not
runtime exceptions from checked exceptions. You can interpret that
sentence to mean "bugs are not recoverable short of fixing the code"
in which case sure, runtime exceptions are not recoverable.

> For example, *all* precondition failures are, from the POV of the
> called function, a programming error (akin to NPE or IAE).

This is correct. NPEs and IAEs are programming errors.

> Whether the root cause is an "internal" or "external" error is irrelevant
> at the point where the issue is detected (where the runtime exception
> just conveys that "the call was inappropriate").

It's not about the root cause. It's about the immediate cause. Passing
null or another illegal argument to a method that isn't ready to
receive it is a programming error, i.e. a bug, and therefore these are
properly runtime exceptions.

> > and it's never fun when a system goes down
> > in production because some random JSON yahoo thought checked
> > exceptions were "icky".
>
> A bug of the application, by the "absent-minded" developer mentioned
> in the previous mail.
> In Java, nothing prevents the throwing of a runtime
> exception; the developer *always* must enclose "foreign" code in a
> "try/catch" block, to protect the system.  There is no need for checked
> exceptions just to remember this (really) simple rule.

Had the JSON library properly used checked exceptions, the programmer
would not have been able to compile code that contained this mistake.
The reason we have strong static typing, unit tests, and checked
exceptions is that programmers are imperfect humans. Let the machines
do the work that machines are good at, including checking whether
errors are properly handled.

> > Lately I've been working in Python, and error handling practice in
> > this community is abominable. The lack of checked exceptions is a
> > large reason why Python code is expected to be unreliable and Python
> > programs routinely dump stack.
>
> I've heard that it's rather the result of "script"-oriented design, or lack
> thereof... :-)

There's more than one reason. :-)


-- 
Elliotte Rusty Harold
elh...@ibiblio.org

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Gary Gregory]

> I never could find a convincing example of "recoverable conditions".

Really? How about typing in a bad server name, user name, or password?
That's a classic recoverable exception (in JDBC land for example).

Gary


On Thu, Jun 29, 2023, 10:11 Gilles Sadowski  wrote:

> Le jeu. 29 juin 2023 à 15:22, Elliotte Rusty Harold
>  a écrit :
> >
> > On Thu, Jun 29, 2023 at 9:07 AM Gilles Sadowski 
> wrote:
> > >
> > > Hello.
> > >
> > > Le jeu. 29 juin 2023 à 14:44, Gary Gregory  a
> écrit :
> >
> > > I agree with the second part assuming the *current* Java
> > > best practices, not because of old APIs that are here to stay
> > > only because of infinite backwards compatibility, and not
> > > because they are deemed perfect.
> >
> >
> > Best practices on this haven't changed since Java 1.0 (Possibly Java
> > 1.0 alpha 3 if I recall versions correctly).
> >
> > Checked exceptions: all errors arising from external input to the
> program.
> > Runtime exceptions: program bugs
>
> A pile of arguments (tally is largely against *any* use of checked
> exceptions):
>
> https://ohadshai.medium.com/its-almost-2020-and-yet-checked-exceptions-are-still-a-thing-bfaa24c8997e
>
> >
> > I don't know which applies here, but 99% of the time this is all you
> > need to consider to figure out whether a checked or runtime exception
> > is called for. There are indeed a lot of old broken APIs that don't
> > follow these simple rules,
>
> The rules which you stated are far from simple because "external"
> depends on the POV (more below).
>
> Commons components like [RNG], [Geometry], [Numbers], [Math]
> only throw unchecked exceptions.
>
> The only rule that ever made sense to me is from J. Bloch's "Effective
> Java":
> ---CUT---
> Use checked exceptions for recoverable conditions and runtime
> exceptions for programming errors
> ---CUT---
>
> I never could find a convincing example of "recoverable conditions".
>
> In any case, Bloch's rule implies that there are several orders of
> magnitude more conditions that should raise an unchecked exception
> that there are for checked ones.
> For example, *all* precondition failures are, from the POV of the
> called function, a programming error (akin to NPE or IAE).
> Whether the root cause is an "internal" or "external" error is irrelevant
> at the point where the issue is detected (where the runtime exception
> just conveys that "the call was inappropriate").
>
> > and it's never fun when a system goes down
> > in production because some random JSON yahoo thought checked
> > exceptions were "icky".
>
> A bug of the application, by the "absent-minded" developer mentioned
> in the previous mail.  In Java, nothing prevents the throwing of a runtime
> exception; the developer *always* must enclose "foreign" code in a
> "try/catch" block, to protect the system.  There is no need for checked
> exceptions just to remember this (really) simple rule.
>
> >
> > Lately I've been working in Python, and error handling practice in
> > this community is abominable. The lack of checked exceptions is a
> > large reason why Python code is expected to be unreliable and Python
> > programs routinely dump stack.
>
> I've heard that it's rather the result of "script"-oriented design, or lack
> thereof... :-)
>
> Regards,
> Gilles
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Gilles Sadowski]

Le jeu. 29 juin 2023 à 15:22, Elliotte Rusty Harold
 a écrit :
>
> On Thu, Jun 29, 2023 at 9:07 AM Gilles Sadowski  wrote:
> >
> > Hello.
> >
> > Le jeu. 29 juin 2023 à 14:44, Gary Gregory  a écrit 
> > :
>
> > I agree with the second part assuming the *current* Java
> > best practices, not because of old APIs that are here to stay
> > only because of infinite backwards compatibility, and not
> > because they are deemed perfect.
>
>
> Best practices on this haven't changed since Java 1.0 (Possibly Java
> 1.0 alpha 3 if I recall versions correctly).
>
> Checked exceptions: all errors arising from external input to the program.
> Runtime exceptions: program bugs

A pile of arguments (tally is largely against *any* use of checked exceptions):
  
https://ohadshai.medium.com/its-almost-2020-and-yet-checked-exceptions-are-still-a-thing-bfaa24c8997e

>
> I don't know which applies here, but 99% of the time this is all you
> need to consider to figure out whether a checked or runtime exception
> is called for. There are indeed a lot of old broken APIs that don't
> follow these simple rules,

The rules which you stated are far from simple because "external"
depends on the POV (more below).

Commons components like [RNG], [Geometry], [Numbers], [Math]
only throw unchecked exceptions.

The only rule that ever made sense to me is from J. Bloch's "Effective
Java":
---CUT---
Use checked exceptions for recoverable conditions and runtime
exceptions for programming errors
---CUT---

I never could find a convincing example of "recoverable conditions".

In any case, Bloch's rule implies that there are several orders of
magnitude more conditions that should raise an unchecked exception
that there are for checked ones.
For example, *all* precondition failures are, from the POV of the
called function, a programming error (akin to NPE or IAE).
Whether the root cause is an "internal" or "external" error is irrelevant
at the point where the issue is detected (where the runtime exception
just conveys that "the call was inappropriate").

> and it's never fun when a system goes down
> in production because some random JSON yahoo thought checked
> exceptions were "icky".

A bug of the application, by the "absent-minded" developer mentioned
in the previous mail.  In Java, nothing prevents the throwing of a runtime
exception; the developer *always* must enclose "foreign" code in a
"try/catch" block, to protect the system.  There is no need for checked
exceptions just to remember this (really) simple rule.

>
> Lately I've been working in Python, and error handling practice in
> this community is abominable. The lack of checked exceptions is a
> large reason why Python code is expected to be unreliable and Python
> programs routinely dump stack.

I've heard that it's rather the result of "script"-oriented design, or lack
thereof... :-)

Regards,
Gilles

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Elliotte Rusty Harold]

On Thu, Jun 29, 2023 at 9:07 AM Gilles Sadowski  wrote:
>
> Hello.
>
> Le jeu. 29 juin 2023 à 14:44, Gary Gregory  a écrit :

> I agree with the second part assuming the *current* Java
> best practices, not because of old APIs that are here to stay
> only because of infinite backwards compatibility, and not
> because they are deemed perfect.


Best practices on this haven't changed since Java 1.0 (Possibly Java
1.0 alpha 3 if I recall versions correctly).

Checked exceptions: all errors arising from external input to the program.
Runtime exceptions: program bugs

I don't know which applies here, but 99% of the time this is all you
need to consider to figure out whether a checked or runtime exception
is called for. There are indeed a lot of old broken APIs that don't
follow these simple rules, and it's never fun when a system goes down
in production because some random JSON yahoo thought checked
exceptions were "icky".

Lately I've been working in Python, and error handling practice in
this community is abominable. The lack of checked exceptions is a
large reason why Python code is expected to be unreliable and Python
programs routinely dump stack.

-- 
Elliotte Rusty Harold
elh...@ibiblio.org

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Gilles Sadowski]

Hello.

Le jeu. 29 juin 2023 à 14:44, Gary Gregory  a écrit :
> [...]
>
> Java makes a clear distinction between checked and unchecked
> exceptions. As a mid-level API, I do not feel Pool should take a
> design POV that is different from Java's intended pattern regarding
> exceptions.

I agree with the second part assuming the *current* Java
best practices, not because of old APIs that are here to stay
only because of infinite backwards compatibility, and not
because they are deemed perfect.
I don't see any relationship between "mid-level API" and
"checked exceptions".
Any reference (of modern code) for it?

>
> The 3.0 API should use checked exceptions

-1

> and the implementation wrap
> unchecked runtime exceptions. This will allow call sites to either
> propagate or catch the one exception.

The one "PoolException" need not be checked, to the same
effect from an application developer POV.

Gilles

>
> [...]

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Gary Gregory]

Thanks for digging in Mark (more below which will just echo my reply to Phil).

On Thu, Jun 29, 2023 at 5:43 AM Mark Thomas  wrote:
>
> On 28/06/2023 14:16, Gary Gregory wrote:
> > Hi All and Phil.
>
> I haven't been that involved in Pool recently but Pool remains a key
> dependency for Tomcat (via DBCP).
>
> > The main driver here was two combine keeping binary compatibility
> > _and_ benefit call sites of the API by _not_ having to catch Exception
> > or throw Exception, which clearly is an anti-pattern. Instead, call
> > sites should deal with domain exceptions, which they can in turn
> > handle. IOW, when I am using IO resources or JDBC resources, I should
> > expect to handle IOException and JDBCException, not Exception, so by
> > extension (more on this below), I would expect to handle the same
> > kinds of exceptions when I am pooling those kinds of resources.
>
> I understand the aim and I am generally in favour of simplification.
>
> However, Pool still throws unchecked exceptions (NoSuchElementException
> and IllegalStateException, possibly others) during normal operations and
> I think that significantly undermines what this change is trying to achieve.

This is a good point for making the API simpler to use and I think
that in the future the implementation should wrap unchecked exceptions
as pool exceptions.

>
> 
>
> > Question: If we were to design Pool from scratch today, would we
> > create APIs with checked or unchecked exceptions?
>
> For Pool I have a slight preference for using checked exceptions
> throughout but I agree that is a larger design change that needs a wider
> discussion.
>
> 
>
> > I did not get the points Phil is trying to make in his examples, I'm
> > not sure they are valid. OTOH, the unit tests do show using the APIs
> > typed with different types of exceptions. Sure, there might be
> > adjustments to make, but I don't see the flaws you seem to think are
> > there. WRT to source changes, I welcome cleaning up my call sites! The
> > debate is valid and I hope we have interesting replies to this thread.
>
> My main concern is not whether Pool should make this change (I think the
> change makes sense) but when Pool should make this change and alongside
> what other changes.
>
> At this point I'm not convinced the cost / benefit of the change in a
> minor release stacks up.

Fair enough, so I am switching my brain to the 3.0 context.

>
> I suspect very few users are simply going to replace the old JAR with
> the new one and take advantage of binary compatibility. They are going
> to update their build dependencies and then compilation is going to
> fail. I don't think users will appreciate that in a minor release
> however much we tell them they are binary compatible.

For some projects, yes of course but in general, I feel you are not
quite right here: Software stacks can be so deep these days that a lot
of dependencies come in transitively, where binary compatibility is
obviously paramount.

>
> To fix the compilation issues and retain existing behaviour (without
> worrying about the unchecked exceptions the Pool does throw during
> normal operations) they are going to have to modify their code so that E
> == Exception. That is going to look like a make-work task as they will
> get zero benefit from it.

The idea is exactly not that. You would type your factory a domain
exception. Typing it as `Exception` would be like typing the domain
class to `Object`.

>
> I think the majority of the benefit of this changes comes if Pool also
> stops throwing unchecked exceptions - as a minimum during normal pooling
> operations.

Agreed, see my other comments.

>
> I'd prefer to see this change deferred to a Pool 3.0 release preceded by
> a wider discussion of the benefits of unchecked vs checked that has
> started in this thread.

We're discussing it now ;-) See my reply to Phil.

Gary

>
> Mark
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Gary Gregory]

On Wed, Jun 28, 2023 at 5:26 PM Phil Steitz  wrote:
>
> On Wed, Jun 28, 2023 at 6:17 AM Gary Gregory  wrote:
>
> > Hi All and Phil.
> >
> > Thank you for the thoughtful reply, Phil.
> >
> > The main driver here was two combine keeping binary compatibility
> > _and_ benefit call sites of the API by _not_ having to catch Exception
> > or throw Exception, which clearly is an anti-pattern. Instead, call
> > sites should deal with domain exceptions, which they can in turn
> > handle. IOW, when I am using IO resources or JDBC resources, I should
> > expect to handle IOException and JDBCException, not Exception, so by
> > extension (more on this below), I would expect to handle the same
> > kinds of exceptions when I am pooling those kinds of resources.
> >
>
> I think I understand.  I am not sure I agree, but I get what you are trying
> to do.  My problem is doing it in a minor release.  I believe you when you
> say that this is "OK" by Commons standards, but breaking source
> compatibility in a low-level, widely used component like [pool] is not very
> nice to our users, IMO.  The "domain exceptions" that I understanding your
> liking for need to be invented (or hopefully as in your examples, found as
> standard exceptions) and code needs to be modified in lots of places
> (unless there is some magic I don't know about, which is entirely possible)
> in order to compile code that directly uses the new version.  I
> respectfully suggest that it would be much better to hold this for a major
> release.  This is also something that should be discussed as we are doing
> now.  I don't see any discussion other than in the JIRA / PRs.
>
> >
> > The idea to convert APIs from throwing exceptions from checked to
> > unchecked seems like a larger design and philosophical change than
> > typing those exceptions with generics, so I stayed away from that.
> >
>
> I get that and agree.  I am not sure it is the best way to go, honestly;
> but here again, I see it as a 3.0 discussion.
>
>
> > Note that changing APIs from checked to unchecked could be made in 2.x
> > _and_ still preserve binary compatibility, just like this RC maintains
> > binary compatibility.
> >
>
> Again, I am not sure binary compat is worth that much for library code that
> is directly used by so many components (such as DBCP).
>
> >
> > I think it is worth talking about what a 3.0 would look like as it
> > would inform how to get there in the best way, and then come back to
> > talking about this RC. A question for a future 3.0 would be whether
> > APIs should throw checked or unchecked exceptions. I think we can
> > safely dismiss any argument as to whether APIs should throw exceptions
> > or not, without discussion; Pool APIs throw exceptions, period, just
> > like any API. As I pointed out above, changing exceptions from checked
> > to unchecked does _not_ break binary compatibility, only source
> > compatibility, so strictly speaking, we don't have to create a major
> > release just for that change. Clearly, a 3.0 would take advantage of
> > removing all deprecated APIs.
> >
> > Question: If we were to design Pool from scratch today, would we
> > create APIs with checked or unchecked exceptions?
> >
>
> I would ask you that :)

Fair enough, so:

Java makes a clear distinction between checked and unchecked
exceptions. As a mid-level API, I do not feel Pool should take a
design POV that is different from Java's intended pattern regarding
exceptions.

The 3.0 API should use checked exceptions and the implementation wrap
unchecked runtime exceptions. This will allow call sites to either
propagate or catch the one exception.

The 2.x code base can be branched and released when the need arises,
no one's asked for one at this time. I am happy to keep maintaining
both 2.x and 3.0.

I will cancel this RC vote and work towards 3.0.

I will experiment with a DBCP 3 from a local experimental branch.

(more below for fun and color)

>
> I am serious.   I don't honestly feel qualified to answer that question
> other than for my own apps.

That's fair as well, but I feel you're more than qualified, maybe
we'll meet in person one day and discuss over a brewski ;-)

> I think getting broad-based feedback somehow
> would be best on this and I honestly trust you more than me to make this
> call.  I will note that we currently throw one unchecked exception - NSE -
> with great frequency and the admittedly loose and maybe "anti-pattern"-ish
> approach in signatures makes that "OK."  (That was the point of one of my
> probably garbled examples below.  Changing borrowObject to advertise a
> checked "FooException" makes it look like if you catch that you catch
> failures; but you don't - you will still get NSE in many cases.)  The de
> facto contract with borrowObject and several other pool methods is that all
> manner of checked and unchecked exceptions may be propagated from either
> the pool itself or the factories being used.  We could do a better job
> documenting the 

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Gilles Sadowski]

Hello.

[Sorry to continue a discussion that should probably go in
its own thread.]

Le jeu. 29 juin 2023 à 11:43, Mark Thomas  a écrit :
>
> On 28/06/2023 14:16, Gary Gregory wrote:
> > Hi All and Phil.
>
> I haven't been that involved in Pool recently but Pool remains a key
> dependency for Tomcat (via DBCP).
>
> > The main driver here was two combine keeping binary compatibility
> > _and_ benefit call sites of the API by _not_ having to catch Exception
> > or throw Exception, which clearly is an anti-pattern. Instead, call
> > sites should deal with domain exceptions, which they can in turn
> > handle. IOW, when I am using IO resources or JDBC resources, I should
> > expect to handle IOException and JDBCException, not Exception, so by
> > extension (more on this below), I would expect to handle the same
> > kinds of exceptions when I am pooling those kinds of resources.
>
> I understand the aim and I am generally in favour of simplification.
>
> However, Pool still throws unchecked exceptions (NoSuchElementException
> and IllegalStateException, possibly others) during normal operations and
> I think that significantly undermines what this change is trying to achieve.

Looking at a commit [1] related to POOL-269 [2], a signature like
  void addObject(K key) throws E, IllegalStateException,
UnsupportedOperationException;
does not qualify as "simple".
1. Unchecked exceptions should not be referred to in the "throws" clause.
2. What is the advantage of a parameterized exception[3] over a common
one (say "PoolException"), and use the standard JDK mechanism of storing
more details in a "cause"[4] ?
3. Unchecked exceptions can always be thrown, so there is the old question
again:  What differentiates, in practice, an error condition that would raise
a checked exception rather than an unchecked one?[5]
4. Protecting from a JVM crash is an application developer's decision. Do we
really want to force all to handle checked exceptions just so that the compiler
can remind the absent-minded (!) that a "try/catch" may be in order?

>
> [...]
>
> To fix the compilation issues and retain existing behaviour (without
> worrying about the unchecked exceptions the Pool does throw during
> normal operations) they are going to have to modify their code so that E
> == Exception. That is going to look like a make-work task as they will
> get zero benefit from it.

How about
 * removing the "throws" clauses
 * reverting the addition of the "E" parameter
?

> I think the majority of the benefit of this changes comes if Pool also
> stops throwing unchecked exceptions - as a minimum during normal pooling
> operations.

How could that be achieved, short of enclosing all function bodies
in a "try/catch" block and rethrowing a checked exception?
For what benefit eventually (as asked above)?

> I'd prefer to see this change deferred to a Pool 3.0 release preceded by
> a wider discussion of the benefits of unchecked vs checked that has
> started in this thread.

AFAICT, unchecked exceptions was the way (a.o. setting an "IOException"
as the cause of an "UncheckedIOException"[6] and rethrowing that one).

Regards,
Gilles

[1] 
https://github.com/apache/commons-pool/pull/143/commits/36ca267abba54dcea11192c61df3ad74a5ac9941
[2] https://issues.apache.org/jira/browse/POOL-269
[3] Is there another library doing that?
[4] 
https://docs.oracle.com/javase/8/docs/api/java/lang/Exception.html#Exception-java.lang.String-java.lang.Throwable-
[5] Mimicking the API of the very early days of the JDK.
[6] https://docs.oracle.com/javase/8/docs/api/java/io/UncheckedIOException.html

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Mark Thomas]

On 28/06/2023 14:16, Gary Gregory wrote:

Hi All and Phil.


I haven't been that involved in Pool recently but Pool remains a key 
dependency for Tomcat (via DBCP).



The main driver here was two combine keeping binary compatibility
_and_ benefit call sites of the API by _not_ having to catch Exception
or throw Exception, which clearly is an anti-pattern. Instead, call
sites should deal with domain exceptions, which they can in turn
handle. IOW, when I am using IO resources or JDBC resources, I should
expect to handle IOException and JDBCException, not Exception, so by
extension (more on this below), I would expect to handle the same
kinds of exceptions when I am pooling those kinds of resources.


I understand the aim and I am generally in favour of simplification.

However, Pool still throws unchecked exceptions (NoSuchElementException 
and IllegalStateException, possibly others) during normal operations and 
I think that significantly undermines what this change is trying to achieve.





Question: If we were to design Pool from scratch today, would we
create APIs with checked or unchecked exceptions?


For Pool I have a slight preference for using checked exceptions 
throughout but I agree that is a larger design change that needs a wider 
discussion.





I did not get the points Phil is trying to make in his examples, I'm
not sure they are valid. OTOH, the unit tests do show using the APIs
typed with different types of exceptions. Sure, there might be
adjustments to make, but I don't see the flaws you seem to think are
there. WRT to source changes, I welcome cleaning up my call sites! The
debate is valid and I hope we have interesting replies to this thread.


My main concern is not whether Pool should make this change (I think the 
change makes sense) but when Pool should make this change and alongside 
what other changes.


At this point I'm not convinced the cost / benefit of the change in a 
minor release stacks up.


I suspect very few users are simply going to replace the old JAR with 
the new one and take advantage of binary compatibility. They are going 
to update their build dependencies and then compilation is going to 
fail. I don't think users will appreciate that in a minor release 
however much we tell them they are binary compatible.


To fix the compilation issues and retain existing behaviour (without 
worrying about the unchecked exceptions the Pool does throw during 
normal operations) they are going to have to modify their code so that E 
== Exception. That is going to look like a make-work task as they will 
get zero benefit from it.


I think the majority of the benefit of this changes comes if Pool also 
stops throwing unchecked exceptions - as a minimum during normal pooling 
operations.


I'd prefer to see this change deferred to a Pool 3.0 release preceded by 
a wider discussion of the benefits of unchecked vs checked that has 
started in this thread.


Mark

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Phil Steitz]

On Wed, Jun 28, 2023 at 6:17 AM Gary Gregory  wrote:

> Hi All and Phil.
>
> Thank you for the thoughtful reply, Phil.
>
> The main driver here was two combine keeping binary compatibility
> _and_ benefit call sites of the API by _not_ having to catch Exception
> or throw Exception, which clearly is an anti-pattern. Instead, call
> sites should deal with domain exceptions, which they can in turn
> handle. IOW, when I am using IO resources or JDBC resources, I should
> expect to handle IOException and JDBCException, not Exception, so by
> extension (more on this below), I would expect to handle the same
> kinds of exceptions when I am pooling those kinds of resources.
>

I think I understand.  I am not sure I agree, but I get what you are trying
to do.  My problem is doing it in a minor release.  I believe you when you
say that this is "OK" by Commons standards, but breaking source
compatibility in a low-level, widely used component like [pool] is not very
nice to our users, IMO.  The "domain exceptions" that I understanding your
liking for need to be invented (or hopefully as in your examples, found as
standard exceptions) and code needs to be modified in lots of places
(unless there is some magic I don't know about, which is entirely possible)
in order to compile code that directly uses the new version.  I
respectfully suggest that it would be much better to hold this for a major
release.  This is also something that should be discussed as we are doing
now.  I don't see any discussion other than in the JIRA / PRs.

>
> The idea to convert APIs from throwing exceptions from checked to
> unchecked seems like a larger design and philosophical change than
> typing those exceptions with generics, so I stayed away from that.
>

I get that and agree.  I am not sure it is the best way to go, honestly;
but here again, I see it as a 3.0 discussion.


> Note that changing APIs from checked to unchecked could be made in 2.x
> _and_ still preserve binary compatibility, just like this RC maintains
> binary compatibility.
>

Again, I am not sure binary compat is worth that much for library code that
is directly used by so many components (such as DBCP).

>
> I think it is worth talking about what a 3.0 would look like as it
> would inform how to get there in the best way, and then come back to
> talking about this RC. A question for a future 3.0 would be whether
> APIs should throw checked or unchecked exceptions. I think we can
> safely dismiss any argument as to whether APIs should throw exceptions
> or not, without discussion; Pool APIs throw exceptions, period, just
> like any API. As I pointed out above, changing exceptions from checked
> to unchecked does _not_ break binary compatibility, only source
> compatibility, so strictly speaking, we don't have to create a major
> release just for that change. Clearly, a 3.0 would take advantage of
> removing all deprecated APIs.
>
> Question: If we were to design Pool from scratch today, would we
> create APIs with checked or unchecked exceptions?
>

I would ask you that :)

I am serious.   I don't honestly feel qualified to answer that question
other than for my own apps.  I think getting broad-based feedback somehow
would be best on this and I honestly trust you more than me to make this
call.  I will note that we currently throw one unchecked exception - NSE -
with great frequency and the admittedly loose and maybe "anti-pattern"-ish
approach in signatures makes that "OK."  (That was the point of one of my
probably garbled examples below.  Changing borrowObject to advertise a
checked "FooException" makes it look like if you catch that you catch
failures; but you don't - you will still get NSE in many cases.)  The de
facto contract with borrowObject and several other pool methods is that all
manner of checked and unchecked exceptions may be propagated from either
the pool itself or the factories being used.  We could do a better job
documenting the ones we throw ourselves but users need to understand what
their factories are going to throw and the cases (almost always) where the
pool propagates rather than swallows / logs.  I am not defending this
setup, but as a user I have never found it that difficult and I have never
really minded the ugliness.  Again, I admit that I am way less concerned
about this kind of thing than most and I understand the desire to make
things cleaner.  But as I keep saying, I really think this is a 3.0
discussion.

>
> The only popular API I know of today that uses unchecked exceptions is
> JPA (like Hibernate). I've used Hibernate _a lot_, and my experience
> there is that just like any API, you eventually have to catch some
> exception somewhere and handle it. With unchecked exceptions, that
> coding decision shows up when you go through a _second_ round of
> compiling and running your sources. IOW, (TTD or not) you write a
> combination of test and main code, run it, and start iterating tests
> and main code, and now you have to deal with 

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Gary Gregory]

Hi All and Phil.

Thank you for the thoughtful reply, Phil.

The main driver here was two combine keeping binary compatibility
_and_ benefit call sites of the API by _not_ having to catch Exception
or throw Exception, which clearly is an anti-pattern. Instead, call
sites should deal with domain exceptions, which they can in turn
handle. IOW, when I am using IO resources or JDBC resources, I should
expect to handle IOException and JDBCException, not Exception, so by
extension (more on this below), I would expect to handle the same
kinds of exceptions when I am pooling those kinds of resources.

The idea to convert APIs from throwing exceptions from checked to
unchecked seems like a larger design and philosophical change than
typing those exceptions with generics, so I stayed away from that.
Note that changing APIs from checked to unchecked could be made in 2.x
_and_ still preserve binary compatibility, just like this RC maintains
binary compatibility.

I think it is worth talking about what a 3.0 would look like as it
would inform how to get there in the best way, and then come back to
talking about this RC. A question for a future 3.0 would be whether
APIs should throw checked or unchecked exceptions. I think we can
safely dismiss any argument as to whether APIs should throw exceptions
or not, without discussion; Pool APIs throw exceptions, period, just
like any API. As I pointed out above, changing exceptions from checked
to unchecked does _not_ break binary compatibility, only source
compatibility, so strictly speaking, we don't have to create a major
release just for that change. Clearly, a 3.0 would take advantage of
removing all deprecated APIs.

Question: If we were to design Pool from scratch today, would we
create APIs with checked or unchecked exceptions?

The only popular API I know of today that uses unchecked exceptions is
JPA (like Hibernate). I've used Hibernate _a lot_, and my experience
there is that just like any API, you eventually have to catch some
exception somewhere and handle it. With unchecked exceptions, that
coding decision shows up when you go through a _second_ round of
compiling and running your sources. IOW, (TTD or not) you write a
combination of test and main code, run it, and start iterating tests
and main code, and now you have to deal with _unexpected_ unchecked
exceptions, unless you've read the Javadoc and/or sources for all the
APIs you're calling, and then wrote _correct_ code. With checked
exceptions, you're forced to make that exception handing decision
upfront or your code won't compile. It's a different experience for
users of the API. I'm not judging it either way, it's just different.

Question: What's the experience we want to give Pool users, the people
who write calls to the API?

I'm not sold 100% on either approach. I consider Pool a mid-level API,
as opposed to a low-level API. Calling a low-level API, I am OK
handling/propagating checked exceptions. I see Pool as a
semi-transparent proxy to the actual API/Objects being pooled, so I am
OK with checked exceptions coming out of Pool just like I am OK with
checked exceptions coming from a file or database API. My point here
is that changing to unchecked API feels to me like raising the
abstraction level of the API. It would mean that we pool and transform
exceptions from checked to unchecked (or, leave them unchecked, or
wrap unchecked domain APIs with Pool unchecked APIs).

What do people think? Is the future checked or unchecked? As is Java
taking on developer-friendly features ("var" for example, and
unfriendly, hello JPMS), are checked exceptions just getting in the
way? Are checked exceptions a step too far in a statically typed
language?

I did not get the points Phil is trying to make in his examples, I'm
not sure they are valid. OTOH, the unit tests do show using the APIs
typed with different types of exceptions. Sure, there might be
adjustments to make, but I don't see the flaws you seem to think are
there. WRT to source changes, I welcome cleaning up my call sites! The
debate is valid and I hope we have interesting replies to this thread.

Thank you for reading all of this!
Gary

On Mon, Jun 26, 2023 at 7:44 PM Phil Steitz  wrote:
>
> Looking more carefully at the extent of impact, I am -1 (non-binding) on
> this change.  I understand what you are trying to do, but a) I don't see
> that the solution actually does it (see comments by Michael in PR) and b)
> it is going to create a lot of pain for users who have to modify factories,
> etc. when in fact many / most of them throw unchecked if any exceptions.  I
> would be +1 for removing the throws entirely in 3.0.  It does not make
> sense to me to have a "generic" checked exception type assigned at the pool
> level.  What the different factory and pool methods throw may be
> different.  Even after your change, for example,  borrowObject throws NSE.
> You are changing the signature to make it look like if you catch "E"
> whatever that is, it will not 

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Phil Steitz]

Looking more carefully at the extent of impact, I am -1 (non-binding) on
this change.  I understand what you are trying to do, but a) I don't see
that the solution actually does it (see comments by Michael in PR) and b)
it is going to create a lot of pain for users who have to modify factories,
etc. when in fact many / most of them throw unchecked if any exceptions.  I
would be +1 for removing the throws entirely in 3.0.  It does not make
sense to me to have a "generic" checked exception type assigned at the pool
level.  What the different factory and pool methods throw may be
different.  Even after your change, for example,  borrowObject throws NSE.
You are changing the signature to make it look like if you catch "E"
whatever that is, it will not go bang.  But it will if you have replaced
"Exception" with a checked exception class. You are just replacing
"Exception" with something that looks "cleaner" but is not.  Clients will
either have to ignore it and just instantiate with "Exception" or create a
gratuitous "FooException" to root a hierarchy of their own making (and
separately catch the NSEs) when in fact what they throw now are standard
exceptions.  Remember pool is very widely used so you are asking  *many*
developers to spend time making changes that many will consider a waste of
time. Is the idea that they all just parameterize with "Exception"?  Can
IDEs do that automatically for them?  Please let's get some input from
downstream users before surprising them with this.

Phil

On Mon, Jun 26, 2023 at 3:55 PM Phil Steitz  wrote:

>
>
> On Mon, Jun 26, 2023 at 3:43 PM Gary Gregory 
> wrote:
>
>> Hi Phil,
>>
>> YW and thank you for the review.
>>
>> Yes, you are right that this is about POOL-269. While binary compatibility
>> is preserved 100%, source compatibility is not. This is one of those rare
>> cases where you can't make an omelette without breaking some eggs ;-)
>> Since
>> binary compatibility is preserved, I don't think this is worth going to a
>> new major release.
>>
>
> We should definitely add a *loud* statement in the release notes then and
> directions for how to work around because I am sure a lot of downstream
> builds will fail due to this as DBCP did.  Is there no way that we can
> somehow keep the source compatability?  Pool is widely used and a lot of
> builds will break IIUC what is going on here.
>
> Phil
>
>>
>> I have POOL projects that will benefit (greatly IMO) from cleaner
>> exception
>> handling and avoid having to throw/propagate Exception or catch/rethrow
>> Exception as a domain Exception, and will now instead be able to use
>> domain
>> specific exception for resources being pooled from the get go.
>>
>> I'll obviously adjust DBCP for this as soon as possible (IOW,
>> post-release).
>>
>> For the other items, I will try and reproduce. My tests builds were ok on
>> Windows 10 and macOS latest with Java 8. Maybe by hardware is too slow or
>> too fast compared to yours, hard to say.
>>
>> Gary
>>
>> On Mon, Jun 26, 2023, 16:53 Phil Steitz  wrote:
>>
>> > Hi Gary, First, thanks for doing this.  There are a lot of good fixes in
>> > here.
>> >
>> > I checked the build, sigs et al on a couple of platforms and did not
>> > find anything major except one item.  I will start with the
>> > show-stopper (IMO) and then the other smaller things.
>> >
>> > 1.  I get compilation failure when I try to compile the latest DBCP
>> > release with this code.  I think it may have something to do with
>> > POOL-269.  Here is one example:
>> >
>> > [*ERROR*]
>> >
>> /Users/psteitz/Downloads/commons-dbcp2-2.9.0-src/src/main/java/org/apache/commons/dbcp2/PoolableConnectionFactory.java:[45,70]
>> > wrong number of type arguments; required 2
>> >
>> > This should not happen in a dot release.
>> >
>> > 2. On MacOS 13.4.1, OpenJDK 20.0.1, I got the following test failure
>> > just one time and can't reproduce:
>> >
>> > java.util.concurrent.ExecutionException:
>> > java.lang.NullPointerException: Cannot invoke
>> >
>> >
>> "org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getIdleObjects()"
>> > because the return value of "java.util.Map.get(Object)" is null
>> >
>> > at
>> > java.base/java.util.concurrent.FutureTask.report(FutureTask.java:122)
>> > at
>> > java.base/java.util.concurrent.FutureTask.get(FutureTask.java:191)
>> > at
>> >
>> org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testConcurrentBorrowAndClear$2(TestGenericKeyedObjectPool.java:1056)
>> > ... 71 more
>> > Caused by: java.lang.NullPointerException: Cannot invoke
>> >
>> >
>> "org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getIdleObjects()"
>> > because the return value of "java.util.Map.get(Object)" is null
>> > at
>> >
>> org.apache.commons.pool2.impl.GenericKeyedObjectPool.addIdleObject(GenericKeyedObjectPool.java:307)
>> > at
>> >
>> org.apache.commons.pool2.impl.GenericKeyedObjectPool.addObject(GenericKeyedObjectPool.java:332)

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Phil Steitz]

On Mon, Jun 26, 2023 at 3:43 PM Gary Gregory  wrote:

> Hi Phil,
>
> YW and thank you for the review.
>
> Yes, you are right that this is about POOL-269. While binary compatibility
> is preserved 100%, source compatibility is not. This is one of those rare
> cases where you can't make an omelette without breaking some eggs ;-) Since
> binary compatibility is preserved, I don't think this is worth going to a
> new major release.
>

We should definitely add a *loud* statement in the release notes then and
directions for how to work around because I am sure a lot of downstream
builds will fail due to this as DBCP did.  Is there no way that we can
somehow keep the source compatability?  Pool is widely used and a lot of
builds will break IIUC what is going on here.

Phil

>
> I have POOL projects that will benefit (greatly IMO) from cleaner exception
> handling and avoid having to throw/propagate Exception or catch/rethrow
> Exception as a domain Exception, and will now instead be able to use domain
> specific exception for resources being pooled from the get go.
>
> I'll obviously adjust DBCP for this as soon as possible (IOW,
> post-release).
>
> For the other items, I will try and reproduce. My tests builds were ok on
> Windows 10 and macOS latest with Java 8. Maybe by hardware is too slow or
> too fast compared to yours, hard to say.
>
> Gary
>
> On Mon, Jun 26, 2023, 16:53 Phil Steitz  wrote:
>
> > Hi Gary, First, thanks for doing this.  There are a lot of good fixes in
> > here.
> >
> > I checked the build, sigs et al on a couple of platforms and did not
> > find anything major except one item.  I will start with the
> > show-stopper (IMO) and then the other smaller things.
> >
> > 1.  I get compilation failure when I try to compile the latest DBCP
> > release with this code.  I think it may have something to do with
> > POOL-269.  Here is one example:
> >
> > [*ERROR*]
> >
> /Users/psteitz/Downloads/commons-dbcp2-2.9.0-src/src/main/java/org/apache/commons/dbcp2/PoolableConnectionFactory.java:[45,70]
> > wrong number of type arguments; required 2
> >
> > This should not happen in a dot release.
> >
> > 2. On MacOS 13.4.1, OpenJDK 20.0.1, I got the following test failure
> > just one time and can't reproduce:
> >
> > java.util.concurrent.ExecutionException:
> > java.lang.NullPointerException: Cannot invoke
> >
> >
> "org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getIdleObjects()"
> > because the return value of "java.util.Map.get(Object)" is null
> >
> > at
> > java.base/java.util.concurrent.FutureTask.report(FutureTask.java:122)
> > at
> > java.base/java.util.concurrent.FutureTask.get(FutureTask.java:191)
> > at
> >
> org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testConcurrentBorrowAndClear$2(TestGenericKeyedObjectPool.java:1056)
> > ... 71 more
> > Caused by: java.lang.NullPointerException: Cannot invoke
> >
> >
> "org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getIdleObjects()"
> > because the return value of "java.util.Map.get(Object)" is null
> > at
> >
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.addIdleObject(GenericKeyedObjectPool.java:307)
> > at
> >
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.addObject(GenericKeyedObjectPool.java:332)
> > at
> >
> org.apache.commons.pool2.KeyedObjectPool.addObjects(KeyedObjectPool.java:136)
> > at
> >
> org.apache.commons.pool2.KeyedObjectPool.addObjects(KeyedObjectPool.java:113)
> > at
> >
> org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testConcurrentBorrowAndClear$0(TestGenericKeyedObjectPool.java:1036)
> > at
> >
> java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:577)
> > at
> > java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
> > at
> >
> java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
> > at
> >
> java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
> > at java.base/java.lang.Thread.run(Thread.java:1623)
> >
> > This one is driving me crazy because the NPE should not be possible
> > due to key registration guarding.  I will keep digging on this.
> >
> > 3.
> >
> > I see this one on every run, both config above and Ubuntu 20.0.4 /
> > OpenJDK 11.0.19
> >
> > Exception in thread "Thread-1305" org.opentest4j.AssertionFailedError
> >
> > at
> > org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:34)
> >
> > at org.junit.jupiter.api.Assertions.fail(Assertions.java:116)
> >
> > at
> >
> org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testClearUnblocksWaiters$3(TestGenericKeyedObjectPool.java:1237)
> >
> > at java.base/java.lang.Thread.run(Thread.java:1623)
> >
> > The test that causes it doesn't fail because it happens in a thread
> > that the test spawns and the test does not 

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Gary Gregory]

Hi Phil,

YW and thank you for the review.

Yes, you are right that this is about POOL-269. While binary compatibility
is preserved 100%, source compatibility is not. This is one of those rare
cases where you can't make an omelette without breaking some eggs ;-) Since
binary compatibility is preserved, I don't think this is worth going to a
new major release.

I have POOL projects that will benefit (greatly IMO) from cleaner exception
handling and avoid having to throw/propagate Exception or catch/rethrow
Exception as a domain Exception, and will now instead be able to use domain
specific exception for resources being pooled from the get go.

I'll obviously adjust DBCP for this as soon as possible (IOW, post-release).

For the other items, I will try and reproduce. My tests builds were ok on
Windows 10 and macOS latest with Java 8. Maybe by hardware is too slow or
too fast compared to yours, hard to say.

Gary

On Mon, Jun 26, 2023, 16:53 Phil Steitz  wrote:

> Hi Gary, First, thanks for doing this.  There are a lot of good fixes in
> here.
>
> I checked the build, sigs et al on a couple of platforms and did not
> find anything major except one item.  I will start with the
> show-stopper (IMO) and then the other smaller things.
>
> 1.  I get compilation failure when I try to compile the latest DBCP
> release with this code.  I think it may have something to do with
> POOL-269.  Here is one example:
>
> [*ERROR*]
> /Users/psteitz/Downloads/commons-dbcp2-2.9.0-src/src/main/java/org/apache/commons/dbcp2/PoolableConnectionFactory.java:[45,70]
> wrong number of type arguments; required 2
>
> This should not happen in a dot release.
>
> 2. On MacOS 13.4.1, OpenJDK 20.0.1, I got the following test failure
> just one time and can't reproduce:
>
> java.util.concurrent.ExecutionException:
> java.lang.NullPointerException: Cannot invoke
>
> "org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getIdleObjects()"
> because the return value of "java.util.Map.get(Object)" is null
>
> at
> java.base/java.util.concurrent.FutureTask.report(FutureTask.java:122)
> at
> java.base/java.util.concurrent.FutureTask.get(FutureTask.java:191)
> at
> org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testConcurrentBorrowAndClear$2(TestGenericKeyedObjectPool.java:1056)
> ... 71 more
> Caused by: java.lang.NullPointerException: Cannot invoke
>
> "org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getIdleObjects()"
> because the return value of "java.util.Map.get(Object)" is null
> at
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.addIdleObject(GenericKeyedObjectPool.java:307)
> at
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.addObject(GenericKeyedObjectPool.java:332)
> at
> org.apache.commons.pool2.KeyedObjectPool.addObjects(KeyedObjectPool.java:136)
> at
> org.apache.commons.pool2.KeyedObjectPool.addObjects(KeyedObjectPool.java:113)
> at
> org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testConcurrentBorrowAndClear$0(TestGenericKeyedObjectPool.java:1036)
> at
> java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:577)
> at
> java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
> at
> java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
> at
> java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
> at java.base/java.lang.Thread.run(Thread.java:1623)
>
> This one is driving me crazy because the NPE should not be possible
> due to key registration guarding.  I will keep digging on this.
>
> 3.
>
> I see this one on every run, both config above and Ubuntu 20.0.4 /
> OpenJDK 11.0.19
>
> Exception in thread "Thread-1305" org.opentest4j.AssertionFailedError
>
> at
> org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:34)
>
> at org.junit.jupiter.api.Assertions.fail(Assertions.java:116)
>
> at
> org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testClearUnblocksWaiters$3(TestGenericKeyedObjectPool.java:1237)
>
> at java.base/java.lang.Thread.run(Thread.java:1623)
>
> The test that causes it doesn't fail because it happens in a thread
> that the test spawns and the test does not wait for the threads it
> starts to finish.  Digging into it, I realize this is my sloppiness as
> I committed this test case.  I will make a PR to fix it.  I don't
> think that it indicates a bug.
>
> 4. I forgot to add since tag on the new version of GKOP clear that
> adds reuseCapacity parm.  Will fix in the same PR.
>
> 5. Small nit.  There is a vestigal paragraph in the release notes
> template that I think should be dropped:
>
>
> No client code changes are required to migrate from versions 2.0-2.3
> to version 2.4.3.
>
> Users of version 1.x should consult the migration guide on the Commons
> Pool web site.
>
> Phil

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Phil Steitz]

Hi Gary, First, thanks for doing this.  There are a lot of good fixes in here.

I checked the build, sigs et al on a couple of platforms and did not
find anything major except one item.  I will start with the
show-stopper (IMO) and then the other smaller things.

1.  I get compilation failure when I try to compile the latest DBCP
release with this code.  I think it may have something to do with
POOL-269.  Here is one example:

[*ERROR*] 
/Users/psteitz/Downloads/commons-dbcp2-2.9.0-src/src/main/java/org/apache/commons/dbcp2/PoolableConnectionFactory.java:[45,70]
wrong number of type arguments; required 2

This should not happen in a dot release.

2. On MacOS 13.4.1, OpenJDK 20.0.1, I got the following test failure
just one time and can't reproduce:

java.util.concurrent.ExecutionException:
java.lang.NullPointerException: Cannot invoke
"org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getIdleObjects()"
because the return value of "java.util.Map.get(Object)" is null

at java.base/java.util.concurrent.FutureTask.report(FutureTask.java:122)
at java.base/java.util.concurrent.FutureTask.get(FutureTask.java:191)
at 
org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testConcurrentBorrowAndClear$2(TestGenericKeyedObjectPool.java:1056)
... 71 more
Caused by: java.lang.NullPointerException: Cannot invoke
"org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getIdleObjects()"
because the return value of "java.util.Map.get(Object)" is null
at 
org.apache.commons.pool2.impl.GenericKeyedObjectPool.addIdleObject(GenericKeyedObjectPool.java:307)
at 
org.apache.commons.pool2.impl.GenericKeyedObjectPool.addObject(GenericKeyedObjectPool.java:332)
at 
org.apache.commons.pool2.KeyedObjectPool.addObjects(KeyedObjectPool.java:136)
at 
org.apache.commons.pool2.KeyedObjectPool.addObjects(KeyedObjectPool.java:113)
at 
org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testConcurrentBorrowAndClear$0(TestGenericKeyedObjectPool.java:1036)
at 
java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:577)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
at 
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
at 
java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
at java.base/java.lang.Thread.run(Thread.java:1623)

This one is driving me crazy because the NPE should not be possible
due to key registration guarding.  I will keep digging on this.

3.

I see this one on every run, both config above and Ubuntu 20.0.4 /
OpenJDK 11.0.19

Exception in thread "Thread-1305" org.opentest4j.AssertionFailedError

at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:34)

at org.junit.jupiter.api.Assertions.fail(Assertions.java:116)

at 
org.apache.commons.pool2.impl.TestGenericKeyedObjectPool.lambda$testClearUnblocksWaiters$3(TestGenericKeyedObjectPool.java:1237)

at java.base/java.lang.Thread.run(Thread.java:1623)

The test that causes it doesn't fail because it happens in a thread
that the test spawns and the test does not wait for the threads it
starts to finish.  Digging into it, I realize this is my sloppiness as
I committed this test case.  I will make a PR to fix it.  I don't
think that it indicates a bug.

4. I forgot to add since tag on the new version of GKOP clear that
adds reuseCapacity parm.  Will fix in the same PR.

5. Small nit.  There is a vestigal paragraph in the release notes
template that I think should be dropped:


No client code changes are required to migrate from versions 2.0-2.3
to version 2.4.3.

Users of version 1.x should consult the migration guide on the Commons
Pool web site.

Phil

On Sat, Jun 24, 2023 at 6:27 PM Gary Gregory  wrote:

> We have fixed quite a few bugs and added some enhancements since
> Apache Commons Pool 2.11.1 was released, so I would like to release
> Apache Commons Pool 2.12.0.
>
> Apache Commons Pool 2.12.0 RC1 is available for review here:
> https://dist.apache.org/repos/dist/dev/commons/pool/2.12.0-RC1
> (svn revision 62626)
>
> The Git tag commons-pool-2.12.0-RC1 commit for this RC is
> e5dae53e0ce1211b40680e7dccf601c3c3897378 which you can browse here:
>
> https://gitbox.apache.org/repos/asf?p=commons-pool.git;a=commit;h=e5dae53e0ce1211b40680e7dccf601c3c3897378
> You may checkout this tag using:
> git clone https://gitbox.apache.org/repos/asf/commons-pool.git
> --branch 
> commons-pool-2.12.0-RC1 commons-pool-2.12.0-RC1
>
> Maven artifacts are here:
>
> https://repository.apache.org/content/repositories/orgapachecommons-1640/org/apache/commons/commons-pool2/2.12.0/
>
> These are the artifacts and their hashes:
>
> #Release SHA-512s
> #Sat Jun 24 21:12:18 EDT 2023
>
> 

Re: [VOTE] Release Apache Commons Pool 2.12.0 based on RC1

[Bruno Kinoshita]

   [x] +1 Release these artifacts

Building OK from tag on

Apache Maven 3.8.5 (3599d3414f046de2324203b78ddcf9b5e4388aa0)
Maven home: /opt/apache-maven-3.8.5
Java version: 17.0.7, vendor: Private Build, runtime:
/usr/lib/jvm/java-17-openjdk-amd64
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "5.15.0-75-generic", arch: "amd64", family:
"unix"

Site reports look good. Only one PMD issue, and it's actually annotated
with NOPMD (maybe on the wrong line of the while-statement?)

Thanks!

Bruno


On Sun, 25 Jun 2023 at 03:27, Gary Gregory  wrote:

> We have fixed quite a few bugs and added some enhancements since
> Apache Commons Pool 2.11.1 was released, so I would like to release
> Apache Commons Pool 2.12.0.
>
> Apache Commons Pool 2.12.0 RC1 is available for review here:
> https://dist.apache.org/repos/dist/dev/commons/pool/2.12.0-RC1
> (svn revision 62626)
>
> The Git tag commons-pool-2.12.0-RC1 commit for this RC is
> e5dae53e0ce1211b40680e7dccf601c3c3897378 which you can browse here:
>
> https://gitbox.apache.org/repos/asf?p=commons-pool.git;a=commit;h=e5dae53e0ce1211b40680e7dccf601c3c3897378
> You may checkout this tag using:
> git clone https://gitbox.apache.org/repos/asf/commons-pool.git
> --branch 
> commons-pool-2.12.0-RC1 commons-pool-2.12.0-RC1
>
> Maven artifacts are here:
>
> https://repository.apache.org/content/repositories/orgapachecommons-1640/org/apache/commons/commons-pool2/2.12.0/
>
> These are the artifacts and their hashes:
>
> #Release SHA-512s
> #Sat Jun 24 21:12:18 EDT 2023
>
> commons-pool2-2.12.0-bin.tar.gz=e16b3a81c98feae6f9855a9bd2f2226dae51558c6e7bb77ee626e58853420ccc59d0943a594bba27ab7147524eca823cac47484e304ebaf14bd724b96bbffc25
>
> commons-pool2-2.12.0-bin.zip=29339f89a8efaa4ad3efbe656d610b8951be1a2a005ba7cb58ae356660331af9d473e8ada8bcdedd3d765c54d1dcfee8779ccb3902f0220de7e92e3039f95b8d
>
> commons-pool2-2.12.0-bom.json=adb3e197d360dc7f53ab116c8fa8b1699d60560fcf977cae613c4cb493168a130ca8041d4ca475e75386f281688819fa5f5111e4aa937b24390a6de8e779f507
>
> commons-pool2-2.12.0-bom.xml=d964e9ec5ed51c10591093bdd0e174f8eb354a447f710e8c3749100fdbda7456e3922846a7190180e5044fe46e571cbc600aaea1b8f64a37c12c5deaa2f1662a
>
> commons-pool2-2.12.0-javadoc.jar=356891b25f2e0367b74a7c4070c26d3cec7a3e608b6e47205e5ffeface590c9717187cb1fa72ebb4c484adaac2c7634ff1944c88282ba9c6551ab5abb58c87f4
>
> commons-pool2-2.12.0-sources.jar=6d955b437496d7af6f94844010a1df15efc04e2b9c15fadc1001777c54a60433570744605b0625a21adf53f03ce9e339809977384c562ad357a98370749c8ee6
>
> commons-pool2-2.12.0-src.tar.gz=eed0575d8357349c908fe8539db2c8ef23234c306f373d203d3d2d9a4ee1ae51cb6bdb2f86163e2296ac90f67a27c7d8cfde239cdfc8ab4966c6239b63f5984e
>
> commons-pool2-2.12.0-src.zip=d8158fd14ee393a99dd0abcb55448b699182a50e0ea114cd3a2681799c9f5e58161f71a2420d0605d1ea39efb08c31be1f7a38531c169fd2c69cc604458ee184
>
> commons-pool2-2.12.0-test-sources.jar=db4fabab1fae77e5dcf8af35397635738c6296ebc25065be9a73725d6b837179c3973ae3ea531aa40056459f011ff90e3c2ef16ad2fa77114dacaa5709e3bf57
>
> commons-pool2-2.12.0-tests.jar=81af180ba6d2a5ce12064c9cc4eda4bc25d072fef55a3dc7ce48506571d40aceb6636c80202c31eff867eb8fd1971a44e0c2c2979019a1e32f43005d70cf2f5e
>
> org.apache.commons_commons-pool2-2.12.0.spdx.json=ed49a8ca7a776ede454f8765f1bd71b5d6a2da35b8bd46ebe930f663127fbd6f248c27059b417ca578665295113ca56a0f0e6486c92436b7b9af7984e3f111db
>
> I have tested this with:
>
> mvn -V -Duser.name=$my_apache_id
> -Dcommons.release-plugin.version=$commons_release_plugin_version
> -Prelease -Ptest-deploy -P jacoco -P japicmp clean package site deploy
>
> using:
>
> Apache Maven 3.9.2 (c9616018c7a021c1c39be70fb2843d6f5f9b8a1c)
> Maven home: /usr/local/Cellar/maven/3.9.2/libexec
> Java version: 1.8.0_372, vendor: Homebrew, runtime:
> /usr/local/Cellar/openjdk@8
> /1.8.0+372/libexec/openjdk.jdk/Contents/Home/jre
> Default locale: en_US, platform encoding: UTF-8
> OS name: "mac os x", version: "13.4.1", arch: "x86_64", family: "mac"
> Darwin gdg-mac-mini.local 22.5.0 Darwin Kernel Version 22.5.0: Thu Jun
>  8 22:22:22 PDT 2023; root:xnu-8796.121.3~7/RELEASE_X86_64 x86_64
>
> Details of changes since 2.11.1 are in the release notes:
>
> https://dist.apache.org/repos/dist/dev/commons/pool/2.12.0-RC1/RELEASE-NOTES.txt
>
> https://dist.apache.org/repos/dist/dev/commons/pool/2.12.0-RC1/site/changes-report.html
>
> Site:
>
> https://dist.apache.org/repos/dist/dev/commons/pool/2.12.0-RC1/site/index.html
> (note some *relative* links are broken and the 2.12.0 directories
> are not yet created - these will be OK once the site is deployed.)
>
> JApiCmp Report (compared to 2.11.1):
>
> https://dist.apache.org/repos/dist/dev/commons/pool/2.12.0-RC1/site/japicmp.html
>
> RAT Report:
>
> https://dist.apache.org/repos/dist/dev/commons/pool/2.12.0-RC1/site/rat-report.html
>
> KEYS:
>   https://www.apache.org/dist/commons/KEYS
>
> Please review the release candidate and vote.
> This