On 23/05/2008, Niall Pemberton <[EMAIL PROTECTED]> wrote: > On Fri, May 23, 2008 at 3:31 PM, Luc Maisonobe <[EMAIL PROTECTED]> wrote: > > Niall Pemberton a écrit : > >> On Thu, May 22, 2008 at 9:35 PM, Luc Maisonobe <[EMAIL PROTECTED]> wrote: > >>> A few comments on this release. > >>> > >>> Typo in the project description in the pom.xml file: replace > >>> "implmentation" with "implementation". > >>> > >>> Extracting files from the commons-chain-1.2-src.tar.gz archive in a > >>> Linux box leads to an all lower case file name for "license-header.txt", > >>> which leads to an error when running "mvn site". Some plugin requires a > >>> mixed case LICENSE-header.txt. > >> > >> Thanks, I fixed the typo and checkstyle config in the trunk: > >> http://svn.apache.org/viewvc?view=rev&revision=659361 > >> > >> Anyone think we need a new RC for this? > > > > No, it is really minor. > > > >> > >>> There are 39 findbugs errors. They don't seem too important. Many are > >>> serialization related (missing serialVersionUID, transient fields) and > >>> many are style related (redeclaration of interfaces from superclass). I > >>> think the errors in ContextBase and web.ChainListener are false > >>> positive. The MTIA_SUSPECT_SERVLET_INSTANCE_FIELD may be more > >>> problematic, I know nothing about servlets so cannot judge this. I'm > >>> attaching the findbug.html report file to this message. > >> > >> I don't see it attached - also I added findbugs to the pom and ran it > >> and didn't see such an error > >> http://svn.apache.org/viewvc?view=rev&revision=659363 > > > > Ooops. I forgot to join the page. Here it is. It was generated by > > version 1.1.1 of the findbugs plugin, and the class files were compiled > > with SunJDK 1.6 on a linux box. > > > Maybe its getting removed by the mailing list, because I still don't > see it. I tried changing the version to 1.1.1 of findbugs and used JDK > 1.6 - but I still don't see it. AnywayI looked up that error here: > > > http://findbugs.sourceforge.net/bugDescriptions.html#MTIA_SUSPECT_SERVLET_INSTANCE_FIELD > > ...so I guess it must be referring to these fields: > > http://people.apache.org/~niallp/chain_1_2_RC2/site/xref/org/apache/commons/chain/web/servlet/ChainProcessor.html#94 > > In thoses cases then this warning is not an issue - since its fine for > all threads to use the same instance variables - three are just the > names of attributes configured for the servlet. CatalogFactory is a > singleton-per-ClassLoader and one instance should be shared by all > threads for the servlet instance. Looking at the code I believe it > "caches" the factory in the servlet to avoid the *synchronized* lookup > in CatalogFactory.getInstance(): >
If the same instance is used by multiple threads, then any instance variables need to be either final, volatile or synchronized. Without one of these, there is no guarantee of memory visibility - thread A can set the value of "catalog" and thread B may never see it. > > http://people.apache.org/~niallp/chain_1_2_RC2/site/xref/org/apache/commons/chain/CatalogFactory.html#178 > > So if I'm looking at the right place then I don't believe this is an > issue - if I'm not looking in the right place then please point me to > the line number(s) your findbugs report is showing in the rc2 xref: > http://people.apache.org/~niallp/chain_1_2_RC2/site/xref/index.html > > Thanks > > > Niall > > > > Luc > >> > >> Niall > >> > >>> I don't cast any vote now, waiting for more knowledgeable people to look > >>> at these servlet issues. > >>> > >>> Luc > >>> > >>> Oliver Heger wrote: > >>>> +1 > >>>> > >>>> Oliver > >>>> > >>>> Niall Pemberton wrote: > >>>>> The main changes since RC1 are that the ant build now works on JDK 1.3 > >>>>> and the Logging dependency has been upgraded to the latest 1.1.1 > >>>>> > >>>>> The artifacts are here: > >>>>> http://people.apache.org/~niallp/chain_1_2_RC2/ > >>>>> > >>>>> SVN Tag: > >>>>> http://svn.apache.org/viewvc/commons/proper/chain/tags/CHAIN_1_2_RC2/ > >>>>> > >>>>> Site: > >>>>> http://people.apache.org/~niallp/chain_1_2_RC2/site/ > >>>>> (note m2 generates relative links, so some don't work - but the site > >>>>> is for info and not included in the release artifacts) > >>>>> > >>>>> Release Notes: > >>>>> http://people.apache.org/~niallp/chain_1_2_RC2/RELEASE-NOTES.txt > >>>>> http://people.apache.org/~niallp/chain_1_2_RC2/site/changes-report.html > >>>>> > >>>>> RAT Report: > >>>>> http://people.apache.org/~niallp/chain_1_2_RC2/site/rat-report.html > >>>>> > >>>>> CLIRR Report: > >>>>> http://people.apache.org/~niallp/chain_1_2_RC2/site/clirr-report.html > >>>>> > >>>>> RC2 has been built with m2 - but m1 and ant builds are available - > details here: > >>>>> http://people.apache.org/~niallp/chain_1_2_RC2/site/building.html > >>>>> > >>>>> Note: Chain is targetted at JDK 1.3, but I built with JDK 1.5 because > >>>>> of the issue with m2 and JDK 1.4 - but I have tested on JDK 1.3 and > >>>>> JDK 1.4 using m1 & ant and JDK 1.5 and JDK 1.6 using m2 > >>>>> > >>>>> Vote is open for 72 hours > >>>>> > >>>>> Thanks in advance for your feedback/votes. > >>>>> > >>>>> Niall > >>>>> > -------------------------------------------------------------------------------------------------------------> > >>>>> > >>>>> [ ] +1 I support this release > >>>>> [ ] +0 I am OK with this release > >>>>> [ ] -0 OK, but.... > >>>>> [ ] -1 I do not support this release > >>>>> > >>>>> --------------------------------------------------------------------- > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
