osdir.com

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [VOTE] [CANCEL] Release Apache Commons Pool2 2.6.1


On Mon, Nov 19, 2018 at 2:48 PM Phil Steitz <phil.steitz@xxxxxxxxx> wrote:

> Sorry ENOTIME, but I remembered that there is a nullsafe
> addIdleObject (see how addObject does it).  In fact, you might just
> replace the manual create and add with just a call to addObject
> itself.  That will also passivate the object before putting it into
> the pool, which is IIRC an invariant (objects put into the idle pool
> are passivated first).
>
> If you want to see the current code blow up, you will have to
> arrange a test with lots of borrows and returns mixed with destroys
> on validate and hope you can get the if test to succeed and then a
> return/borrow sequence before the create.
>

Like this then:

diff --git
a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
index 0575f7e..6d81dbc 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
@@ -920,8 +920,7 @@
             // In case there are already threads waiting on something in
the pool
             // (e.g. idleObjects.takeFirst(); then we need to provide them
a fresh instance.
             // Otherwise they will be stuck forever (or until timeout)
-            PooledObject<T> freshPooled = create();
-            idleObjects.put(freshPooled);
+            addObject();
         }
     }

But causes a failure:

[INFO] Running org.apache.commons.pool2.impl.TestAbandonedObjectPool
[ERROR] Tests run: 6, Failures: 1, Errors: 0, Skipped: 0, Time elapsed:
12.253 s <<< FAILURE! - in
org.apache.commons.pool2.impl.TestAbandonedObjectPool
[ERROR]
testAbandonedInvalidate(org.apache.commons.pool2.impl.TestAbandonedObjectPool)
Time elapsed: 3.668 s  <<< FAILURE!
java.lang.AssertionError: expected:<5> but was:<4>
        at
org.apache.commons.pool2.impl.TestAbandonedObjectPool.testAbandonedInvalidate(TestAbandonedObjectPool.java:202)

Maybe this is due to my busy CPU, not sure.

Gary


>
> Phil
>
> On 11/19/18 2:31 PM, Gary Gregory wrote:
> > A unit test? Yes please! :-)
> >
> > Gary
> >
> > On Mon, Nov 19, 2018 at 1:23 PM Mark Struberg <struberg@xxxxxxxx.invalid
> >
> > wrote:
> >
> >> +1 for the null check.
> >>
> >> Do you want to re-open the ticket and create a patch?
> >>
> >> I've created a unit test which proves my original problem with the
> >> dead-lock.
> >> So any improvement should be rather on the safe side from here on.
> >>
> >>
> >> Regarding the RC: this is really not needed anymore when working with
> GIT
> >> as nothing gets pushed/released to the main repository! See the config
> >> changes I did to the maven-release-plugin.
> >>
> >> txs and LieGrue,
> >> strub
> >>
> >>
> >>
> >>> Am 19.11.2018 um 16:43 schrieb Phil Steitz <phil.steitz@xxxxxxxxx>:
> >>>
> >>> On 11/19/18 8:19 AM, Gary Gregory wrote:
> >>>> On Mon, Nov 19, 2018 at 6:04 AM Rob Tompkins <chtompki@xxxxxxxxx>
> >> wrote:
> >>>>> I’d be happy to roll the release if we get master to where you want
> >> it.
> >>>> IMO, we should integrate the recent PR I mentioned and roll RC3. Note
> >> that this vote subject thread did not contain an RC number. Sticking to
> the
> >> usual process would be less troublesome IMO.
> >>> I have not had a chance to fully review and am not really active in
> >> [pool] any more, but I did notice that the fix for POOL-356 is missing a
> >> null check between these two added statements:
> >>>   PooledObject<T> freshPooled = create();
> >>> idleObjects.put(freshPooled);
> >>>
> >>> create() can return null and while in general it won't in this
> >> activation context, given the lack of sync control, it is possible that
> a
> >> return hits between the if test and execution resulting in no capacity
> to
> >> create.
> >>> I also notice some system.outs made it into the test code in one of the
> >> commits related to POOL-340.
> >>> Phil
> >>>> Gary
> >>>>
> >>>>
> >>>>> Cheers,
> >>>>> -Rob
> >>>>>
> >>>>>> On Nov 19, 2018, at 7:18 AM, Mark Struberg
> <struberg@xxxxxxxx.INVALID
> >>>>> wrote:
> >>>>>> Oki, I now see what you mean.
> >>>>>>
> >>>>>> We actually have 3 source zips now.
> >>>>>>
> >>>>>> .src.zip
> >>>>>> .source-release.zip
> >>>>>> src.jar
> >>>>>>
> >>>>>> That's a mess.
> >>>>>>
> >>>>>> There should only be 2:
> >>>>>> * source-release.zip is the official ASF packages whole build
> sources.
> >>>>> This includes the pom, build structure etc.
> >>>>>> * src.jar is the sources which are automatically downloaded by the
> >> IDEs
> >>>>> for debugging purpose.
> >>>>>> We have both of them because commons-pool2 is a single-module
> project.
> >>>>>> And yes, we need both of them. What we do not need is the src.zip. I
> >>>>> have no clue yet where this comes from but it shouldn't be here.
> >>>>>> The good news:
> >>>>>> By leveraging native GIT we now can simply a.) drop the maven
> stating
> >>>>> repo in repository.a.o and b.) drop the release branch and tag from
> my
> >>>>> github account and re-roll the release without any weird RC hacks.
> >>>>>> Will do that,
> >>>>>> * fix the maven setup
> >>>>>> * happy to also include the new ticket
> >>>>>> * re-roll the release this afternoon.
> >>>>>>
> >>>>>> LieGrue,
> >>>>>> strub
> >>>>>>
> >>>>>>> Am 16.11.2018 um 23:10 schrieb Romain Manni-Bucau <
> >>>>> rmannibucau@xxxxxxxxx>:
> >>>>>>> Le ven. 16 nov. 2018 22:54, Gary Gregory <garydgregory@xxxxxxxxx>
> a
> >>>>> écrit :
> >>>>>>>> On Fri, Nov 16, 2018 at 2:32 PM Romain Manni-Bucau <
> >>>>> rmannibucau@xxxxxxxxx>
> >>>>>>>> wrote:
> >>>>>>>>
> >>>>>>>>> Le ven. 16 nov. 2018 21:23, Gary Gregory <garydgregory@xxxxxxxxx
> >
> >> a
> >>>>>>>> écrit
> >>>>>>>>> :
> >>>>>>>>>
> >>>>>>>>>> On Wed, Nov 14, 2018 at 8:59 AM Mark Struberg
> >>>>>>>> <struberg@xxxxxxxx.invalid
> >>>>>>>>>> wrote:
> >>>>>>>>>>
> >>>>>>>>>>> Oki, now the full VOTE text!
> >>>>>>>>>>>
> >>>>>>>>>>> I'd like to call a VOTE on releasing Apache Commons pool2 2.6.1
> >>>>>>>>>>> The release was run with JDK-1.7 to ensure Java7 compatibility.
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> The ASF staging repository is at
> >>>>>>>>>>>
> >>
> https://repository.apache.org/content/repositories/orgapachecommons-1396/
> >>>>>>>>>>> The source zip is at
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>
> https://repository.apache.org/content/repositories/orgapachecommons-1396/org/apache/commons/commons-pool2/2.6.1/
> >>>>>>>>>>> The sha1 of the source-release zip is
> >>>>>>>>>>> 17b01d1e776b7e2b9987b665e1b4e456c02ffa1c
> >>>>>>>>>>> The sha512 is
> >>>>>>>>>>>
> >>
> 982275c963c09e11dd38a3b6621f2a67bab42b6744a1629ab97b7323208b31730b756a7d5bc6dabee54ba0e9f72c8296904f36919fd421fee8e59786c587c388
> >>>>>>>>>> For me:
> >>>>>>>>>>
> >>>>>>>>>> $ sha512sum commons-pool2-2.6.1-src.zip
> >>>>>>>>>>
> >>>>>>>>>>
> >>
> 2b95b00a22bf72a7cdf77f2e40796d126b4a0d7b669564b8b04cd0c884252acd3dac356fe55a9fdaadd4767e13eef560995989cb2d39f862f8d3b7e1d06c773e
> >>>>>>>>>> *commons-pool2-2.6.1-src.zip
> >>>>>>>>>>
> >>>>>>>>>> Which is not what you list above. Please advise.
> >>>>>>>>>>
> >>>>>>>>> Src vs source-release?
> >>>>>>>>>
> >>>>>>>> That's the problem with inventing a new release process... why do
> we
> >>>>> have
> >>>>>>>> BOTH:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>
> https://repository.apache.org/content/repositories/orgapachecommons-1396/org/apache/commons/commons-pool2/2.6.1/commons-pool2-2.6.1-src.zip
> >>>>>>>> AND
> >>>>>>>>
> >>>>>>>>
> >>
> https://repository.apache.org/content/repositories/orgapachecommons-1396/org/apache/commons/commons-pool2/2.6.1/commons-pool2-2.6.1-source-release.zip
> >>>>>>>> And more importantly why are they _different_? Which one will be
> >> used
> >>>>> in
> >>>>>>>> the dist/release area?
> >>>>>>>>
> >>>>>>> Looks like pool didnt do its homework and kept the old assembly
> >> (src),
> >>>>>>> source-release comes from the parent and is likely the one to keep
> >> IMHO
> >>>>>>>
> >>>>>>>> Gary
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> Gary
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>> I added my KEY (struberg at apache.org) to our dist KEYS file
> >>>>>>>>>>> https://dist.apache.org/repos/dist/release/commons/KEYS
> >>>>>>>>>>>
> >>>>>>>>>>> I've created the release in a GIT manner and pushed the
> according
> >>>>>>>>> changes
> >>>>>>>>>>> to my ASF-linked github repo
> >>>>>>>>>>>
> >>>>>>>>>>>
> >> https://github.com/struberg/commons-pool/tree/release_branch_2.6.1
> >>>>>>>>>>> the sha1 of the commit is
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>
> https://github.com/struberg/commons-pool/commit/c910171d9d8c8f5f895b7d18381fc03a51b2a019
> >>>>>>>>>>> the tag is
> >>>>>>>>>>>
> >> https://github.com/struberg/commons-pool/tree/commons-pool2-2.6.1
> >>>>>>>>>>> c910171
> >>>>>>>>>>> <
> >>
> https://github.com/struberg/commons-pool/tree/commons-pool2-2.6.1c910171
> >>>>>>>>>>> This will get pushed to the ASF cannonical repo once the VOTE
> >>>>>>>> succeeds.
> >>>>>>>>>>> Site will be updated once the release has passed.
> >>>>>>>>>>>
> >>>>>>>>>>> Please VOTE:
> >>>>>>>>>>>
> >>>>>>>>>>> [+1] go ship it!
> >>>>>>>>>>> [+0] meh, I don't care
> >>>>>>>>>>> [-1] stop there is a ${showstopper} (that means something
> >>>>> _important_
> >>>>>>>>> is
> >>>>>>>>>>> missing!)
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> Here is my own +1
> >>>>>>>>>>> checked:
> >>>>>>>>>>> * signature
> >>>>>>>>>>> * hashes
> >>>>>>>>>>> * LICENSE
> >>>>>>>>>>> * NOTICE
> >>>>>>>>>>> * rat
> >>>>>>>>>>> * builds fine with various JDKs
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> LieGrue,
> >>>>>>>>>>> strub
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>> Am 14.11.2018 um 10:13 schrieb Mark Struberg
> >>>>>>>>> <struberg@xxxxxxxx.INVALID
> >>>>>>>>>>>> :
> >>>>>>>>>>>>
> >>>>>>>>>>>> PS: I've created the release in a GIT manner and pushed the
> >>>>>>>> according
> >>>>>>>>>>> changes to my ASF-linked github repo
> >> https://github.com/struberg/commons-pool/tree/release_branch_2.6.1
> >>>>>>>>>>>> the sha1 of the commit is
> >>>>>>>>>>>>
> >>
> https://github.com/struberg/commons-pool/commit/c910171d9d8c8f5f895b7d18381fc03a51b2a019
> >>>>>>>>>>>> the tag is
> >>>>>>>>>>>>
> >> https://github.com/struberg/commons-pool/tree/commons-pool2-2.6.1
> >>>>>>>>>>>> c910171
> >>>>>>>>>>>>
> >>>>>>>>>>>> This will get pushed to the ASF cannonical repo once the VOTE
> >>>>>>>>> succeeds.
> >>>>>>>>>>>> Yay, this is the way GIT works and before someone not familiar
> >> with
> >>>>>>>>> GIT
> >>>>>>>>>>> screams that this is not hosted on ASF: This got discussed on
> the
> >>>>>>>> board
> >>>>>>>>>>> level a long time ago (when we did DeltaSpike and CouchDB as
> the
> >>>>> very
> >>>>>>>>>> first
> >>>>>>>>>>> GIT repos at the ASF) and is perfectly fine as all this is
> based
> >> on
> >>>>>>>>>>> cryptographically strong steps.
> >>>>>>>>>>>> LieGrue,
> >>>>>>>>>>>> strub
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>> Am 14.11.2018 um 09:17 schrieb Mark Struberg
> >>>>>>>>>> <struberg@xxxxxxxx.INVALID
> >>>>>>>>>>>> :
> >>>>>>>>>>>>> Hi folks!
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I'm currently preparing the release for commons-pool2-2.6.1
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> So far I did
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> * fix the missing parts in changes.xml
> >>>>>>>>>>>>> * generate + copy the RELEASE_NOTES
> >>>>>>>>>>>>> * run the maven release (after fixing the setup...)
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> The ASF staging repository is at
> >>>>>>>>>>>>>
> >>
> https://repository.apache.org/content/repositories/orgapachecommons-1396/
> >>>>>>>>>>>>> The source zip is at
> >>>>>>>>>>>>>
> >>
> https://repository.apache.org/content/repositories/orgapachecommons-1396/org/apache/commons/commons-pool2/2.6.1/
> >>>>>>>>>>>>> The sha1 of the source-release zip is
> >>>>>>>>>>> 17b01d1e776b7e2b9987b665e1b4e456c02ffa1c
> >>>>>>>>>>>>> The sha512 is
> >>
> 982275c963c09e11dd38a3b6621f2a67bab42b6744a1629ab97b7323208b31730b756a7d5bc6dabee54ba0e9f72c8296904f36919fd421fee8e59786c587c388
> >>>>>>>>>>>>> I added my KEY (struberg at apache.org) to our dist KEYS
> file
> >>>>>>>>>>>>> https://dist.apache.org/repos/dist/release/commons/KEYS
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I will now continue with the follow up steps and then call an
> >>>>>>>>> official
> >>>>>>>>>>> VOTE.
> >>>>>>>>>>>>> Please let me know if something went wrong so far!
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> LieGrue,
> >>>>>>>>>>>>> strub
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >> ---------------------------------------------------------------------
> >>>>>>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
> >>>>>>>>>>>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
> >>>>>>>>>>>>>
> >>>>>>>>>>>>
> >> ---------------------------------------------------------------------
> >>>>>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
> >>>>>>>>>>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>> ---------------------------------------------------------------------
> >>>>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
> >>>>>>>>>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
> >>>>>>
> ---------------------------------------------------------------------
> >>>>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
> >>>>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
> >>>>>>
> >>>>> ---------------------------------------------------------------------
> >>>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
> >>>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
> >>>>>
> >>>>>
> >>>
> >>> ---------------------------------------------------------------------
> >>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
> >>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
> >> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
> >>
> >>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
>
>