commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [pool] 1.5-RC1 available for review
Date Sat, 30 May 2009 02:05:46 GMT
On 30/05/2009, Phil Steitz <phil.steitz@gmail.com> wrote:
> sebb wrote:
>
> > On 29/05/2009, Phil Steitz <phil.steitz@gmail.com> wrote:
> >
> >
> > > The files are here:
> > >
> http://people.apache.org/~psteitz/commons-pool-1.5-RC1/
> > >
> > >
> >
> > findbugs filter and license-header.txt are missing from source archives.
> >
> >
>  This is intended.  The associated reports are not enabled in the release
> pom, so there is no reason to distribute these files with the release.
>

OK.

> > RELEASE-NOTES.txt is missing from the SVN tag.
> >
> >
>  This is a generated file.  I guess I could check in before making the tag,
> but then it would not be generated from the tag itself.

OK, I did not realise it was auto generated.

> > NOTICE.txt has 2008.
> >
> >
>  Good catch.  Thanks!
>
> > I'm not sure why, but the $Id: SVN tags in some of the XML files
> > differ between the source file and a checkout of the SVN tag. This
> > should not happen if the tag is used to create the build.
> >
> >
>  The release was generated from the tag.  Which files?

build.xml:
SVN: <!-- $Id: build.xml 777746 2009-05-22 23:52:23Z sebb $ -->
SRC: <!-- $Id: build.xml 779814 2009-05-29 01:40:51Z psteitz $ -->

downloads.xml
examples.xml
index.xml

overview.html & package.html
impl/package.html

SVN info for the tag shows:

Last Changed Author: psteitz
Last Changed Rev: 779814
Last Changed Date: 2009-05-29 02:40:51 +0100 (Fri, 29 May 2009)

> > It would be nice if the sources jar manifest had Spec. and Impl. entries.
> >
> >
>  Looks like you fixed this.  Thanks!
>
> > Ant test works OK on Java 1.3.1
> >
> > Mvn test (Java 1.6.0) reports
> >
> > [WARNING] Using platform encoding (Cp1252 actually) to copy filtered
> > resources, i.e. build is platform dependent!
> >
> > Also the test fails for me:
> >
> > Failed tests:
> >
> testBorrowObjectFairness(org.apache.commons.pool.impl.TestGenericObjectPool)
> >  Time elapsed: 2.468 sec  <<< FAILURE!
> > junit.framework.AssertionFailedError
> >        at junit.framework.Assert.fail(Assert.java:47)
> >        at junit.framework.Assert.fail(Assert.java:53)
> >        at
> org.apache.commons.pool.impl.TestGenericObjectPool.testBorrowObjectFairness(TestGenericObjectPool.java:1434)
> >
> > The Test case ought to be a bit more explicit as to why the test failed.
> >
> > I suspect it may be an error in the unit-test code - the TestThread
> > class has various fields that are accessed across threads without any
> > synchronisation.
> >
> >
>  I don't think the TestThread instance fields are accessed across threads.

The _failed and _complete booleans are set in one thread and read in another.
These need to be made volatile.

Likewise the other class variables in TestThread, however these are
set before the start() method and not accessed from the main thread
again.

> The test case is timing sensitive, as indicated in the comment.  That said,
> this is a new test case, testing new code, so we need to find out exactly
> what is going on.

I've added code to save the throwable (or generate one).

>  IIUC what the test is trying to do (Mark can correct me if I am wrong), it
> is launching 500 threads, numbered by their array indices,  with 10 ms
> delays between starts and they confirming that they get served in start
> order.  The way that it confirms order is by leveraging the fact that the
> test factory produces numbered objects, 0, 1, 2, ...., maxActive - 1.  So
> thread i should get object i mod maxActive.  That is what is being tested in
> the line you have narrowed the failure to.
>

It does seem to be a timing issue; if the delay in the startup loop is
reduced, the error happens more readily, and if it is increased it
happens less often. Perhaps it needs to be increased to 20ms from
10ms.

>
> >
> >
> > >  The tag is here:
> > >
> http://svn.apache.org/repos/asf/commons/proper/pool/tags/POOL_1_5_RC1/
> > >
> > >
> >
> > NOTICE.txt says:
> >
> > Copyright 1999-2008 The Apache Software Foundation
> >
> > I think this does need to be fixed.
> >
> > DOAP file does not have an AL header (not a blocker, I'll fix trunk)
> >
> >
>  Thanks!
>
>  Thanks for reviewing and for your help with the issues.
>
>  Phil
>
>
> >
> >
> > >  Have at it!
> > >
> > >  Thanks!
> > >
> > >  Phil
> > >
> > >
> ---------------------------------------------------------------------
> > >  To unsubscribe, e-mail:
> dev-unsubscribe@commons.apache.org
> > >  For additional commands, e-mail: dev-help@commons.apache.org
> > >
> > >
> > >
> > >
> >
> >
> ---------------------------------------------------------------------
> > To unsubscribe, e-mail:
> dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
> >
>
>
> ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>  For additional commands, e-mail: dev-help@commons.apache.org
>
>

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


Mime
View raw message