You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by rj...@apache.org on 2011/11/10 22:46:33 UTC

svn commit: r1200582 - /tomcat/tc6.0.x/trunk/STATUS.txt

Author: rjung
Date: Thu Nov 10 21:46:32 2011
New Revision: 1200582

URL: http://svn.apache.org/viewvc?rev=1200582&view=rev
Log:
Add proposal.

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1200582&r1=1200581&r2=1200582&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Thu Nov 10 21:46:32 2011
@@ -96,6 +96,12 @@ PATCHES PROPOSED TO BACKPORT:
   -1:
     kkolinko: +1200004, needs documentation, some unresolved issues in Re:r1199980 thread
 
+* Fix setting of some standard attributes on cluster managers.
+  Backport of r1200555 from trunk.
+  http://people.apache.org/~rjung/patches/tc6-cluster-fix-attribute-setting.patch
+  +1: rjung
+  -1:
+
 PATCHES/ISSUES THAT ARE STALLED
 
 * Backport JSP unloading patch (BZ48358).



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


Re: svn commit: r1200582 - /tomcat/tc6.0.x/trunk/STATUS.txt

Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/11/11 Rainer Jung <ra...@kippdata.de>:
> On 10.11.2011 15:08, Konstantin Kolinko wrote:
>>
>> 2011/11/11<rj...@apache.org>:
>>>
>>> Author: rjung
>>> Date: Thu Nov 10 21:46:32 2011
>>> New Revision: 1200582
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1200582&view=rev
>>> Log:
>>> Add proposal.
>>>
>>> Modified:
>>>    tomcat/tc6.0.x/trunk/STATUS.txt
>>>
>>> +* Fix setting of some standard attributes on cluster managers.
>>> +  Backport of r1200555 from trunk.
>>> +
>>>  http://people.apache.org/~rjung/patches/tc6-cluster-fix-attribute-setting.patch
>>> +  +1: rjung
>>> +  -1:
>>> +
>>
>> What about property change listeners and other side effects? Similar
>> previous changes in trunk:
>> http://svn.apache.org/viewvc?view=revision&revision=1175155
>>
>> I think that the same setters are used when configuring the object
>> through Digester and there are no listeners in a newly created object.
>> So unlikely there are side effects.
>
> It does. I was curious about the exact stacks where the Manager element in
> the Cluster in server.xml vs. Manager in Context in context.xml kick in and
> indeed the setters are called by the Digester.
>
> So probably it would even be better to also use the setter for
> maxActiveSessions.
>
>> Calling  setRandomFile() looks especially suspicious,
>> but anyway if it is never called it is called again at the top of
>> ManagerBase.getRandomBytes().
>
> Yes and it seems safe to be called multiple times.
>
> So I'll also switch maxActiveSessions to calling the setter in trunk/tc 7
> and add that to the proposed TC 6 patch.
>

1) There is a difference whether it creates ManagerBase.randomIS now
or only on the next call to getRandomBytes()


2) I think there is a bug in setRandomFile() implementation.

Note that there is default value for devRandomSource. Then the code does:
                File f=new File( devRandomSource );
                if( ! f.exists() ) return;
...
                devRandomSource = null;

The following two things are wrong:
A.  new File(null) will result in an NPE.
B.  On Windows (or any other configurations where the file does not
exist) it will repeatedly call "new File()" and f.exists() on every
call to getRandomBytes(), wasting IO

I think it should
- ignore the call if the argument to setRandomFile() is null
- set devRandomSource to null if the file does not exist

Because of A. if there was IOException when opening the file you will
result with NPE when calling the setter on a clone.

Best regards,
Konstantin Kolinko

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


Re: svn commit: r1200582 - /tomcat/tc6.0.x/trunk/STATUS.txt

Posted by Rainer Jung <ra...@kippdata.de>.
On 10.11.2011 15:08, Konstantin Kolinko wrote:
> 2011/11/11<rj...@apache.org>:
>> Author: rjung
>> Date: Thu Nov 10 21:46:32 2011
>> New Revision: 1200582
>>
>> URL: http://svn.apache.org/viewvc?rev=1200582&view=rev
>> Log:
>> Add proposal.
>>
>> Modified:
>>     tomcat/tc6.0.x/trunk/STATUS.txt
>>
>> +* Fix setting of some standard attributes on cluster managers.
>> +  Backport of r1200555 from trunk.
>> +  http://people.apache.org/~rjung/patches/tc6-cluster-fix-attribute-setting.patch
>> +  +1: rjung
>> +  -1:
>> +
>
> What about property change listeners and other side effects? Similar
> previous changes in trunk:
> http://svn.apache.org/viewvc?view=revision&revision=1175155
>
> I think that the same setters are used when configuring the object
> through Digester and there are no listeners in a newly created object.
> So unlikely there are side effects.

It does. I was curious about the exact stacks where the Manager element 
in the Cluster in server.xml vs. Manager in Context in context.xml kick 
in and indeed the setters are called by the Digester.

So probably it would even be better to also use the setter for 
maxActiveSessions.

> Calling  setRandomFile() looks especially suspicious,
> but anyway if it is never called it is called again at the top of
> ManagerBase.getRandomBytes().

Yes and it seems safe to be called multiple times.

So I'll also switch maxActiveSessions to calling the setter in trunk/tc 
7 and add that to the proposed TC 6 patch.

Regards,

Rainer


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


Re: svn commit: r1200582 - /tomcat/tc6.0.x/trunk/STATUS.txt

Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/11/11  <rj...@apache.org>:
> Author: rjung
> Date: Thu Nov 10 21:46:32 2011
> New Revision: 1200582
>
> URL: http://svn.apache.org/viewvc?rev=1200582&view=rev
> Log:
> Add proposal.
>
> Modified:
>    tomcat/tc6.0.x/trunk/STATUS.txt
>
> +* Fix setting of some standard attributes on cluster managers.
> +  Backport of r1200555 from trunk.
> +  http://people.apache.org/~rjung/patches/tc6-cluster-fix-attribute-setting.patch
> +  +1: rjung
> +  -1:
> +

What about property change listeners and other side effects? Similar
previous changes in trunk:
http://svn.apache.org/viewvc?view=revision&revision=1175155

I think that the same setters are used when configuring the object
through Digester and there are no listeners in a newly created object.
So unlikely there are side effects.


Calling  setRandomFile() looks especially suspicious,
but anyway if it is never called it is called again at the top of
ManagerBase.getRandomBytes().

Best regards,
Konstantin Kolinko

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