You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Chris R <ch...@gmail.com> on 2012/07/21 22:32:52 UTC
atomicity problem in SecurityUtil.java
Hi,
I am looking at SecurityUtil.java (
http://www.docjar.com/html/api/org/apache/catalina/security/SecurityUtil.java.html)
and it seems that there may be a problem there. More specifically, in the
execute method, getAttribute of Globals.SUBJECT_ATTR is obtained on a
session
and then it is set in a non-atomic fashion.
http://stackoverflow.com/questions/616601/is-httpsession-thread-safe-are-set-get-attribute-thread-safe-operations/616723#616723
says that the get and set should be synchronized on a session. Is this
going to be a problem? Should I file a bug?
Thanks,
Chris.
Re: atomicity problem in SecurityUtil.java
Posted by Mark Thomas <ma...@apache.org>.
Chris R <ch...@gmail.com> wrote:
>Mark,
>
>Thank you for your "kind" response.
>
>>> and it seems that there may be a problem there. More specifically, in the
>>> execute method, getAttribute of Globals.SUBJECT_ATTR is obtained on a
>>> session and then it is set in a non-atomic fashion.
>>
>> While that statement is true, you have not explained why this is a
>> problem in this particular case.
>>
> What more explanation is needed?
So far you have only provide a theoretical explanation of why there might be a problem. You need to provide a concrete example of how a problem could occur given how Tomcat uses this attribute. The "given how Tomcat uses this attribute" part is important. This is an internal attribute that is only used by Tomcat. Therefore whether or not there is a problem here is highly dependent on how Tomcat uses it.
>> Having looked at this particular code, I see some potential
>> opportunities for simplifying things but no chance of a threading
>> problem. If you analysis reaches a different conclusion, please share
>> it.
>Why don't you see a threading problem?
Because I have looked at how the attribute is used.
> Is it because multiple threads cannot access this code?
> Is it because a lost update is not a problem?
> Is it because the session is not shared?
Those are certainly possible reasons for this not being an issue. It could be one of those, it could be something else.
> Instead of a blanket "I do not see a threading problem", provide more
> details so that other new contributors understand and can also help in
> contributing to the project in the future.
My aim is not to provide you with the answer but to point you in the right direction so you can figure out the answer for yourself. That way you'll learn a little more about Tomcat and answer will still be in the archives for future contributors. Also, if you spot something I have missed I have an opportunity to learn something.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: atomicity problem in SecurityUtil.java
Posted by Chris R <ch...@gmail.com>.
Mark,
Thank you for your "kind" response.
> and it seems that there may be a problem there. More specifically, in the
> > execute method, getAttribute of Globals.SUBJECT_ATTR is obtained on a
> > session
> > and then it is set in a non-atomic fashion.
>
> While that statement is true, you have not explained why this is a
> problem in this particular case.
>
> What more explanation is needed? Two threads having access to the same
session object
can clobber the attribute. Both threads (A and B) do a getAttribute, find
the attribute to be null and create a subject.
One of them (A) sets the attribute with a subject with principal x. In the
interim, a third thread (C) does a getAttribute,
subject is not null and proceeds with using principal x. Now thread B gets
scheduled and sets the subject attribute with principal y.
Now, aren't the semantics inconsistent between B and C? After C had
obtained the subject, the subject is guaranteed to not change.
Having looked at this particular code, I see some potential
> opportunities for simplifying things but no chance of a threading
> problem. If you analysis reaches a different conclusion, please share it.
>
Why don't you see a threading problem? Is it because multiple threads
cannot access this code?
Is it because a lost update is not a problem? Is it because the session is
not shared?
Instead of a blanket "I do not see a threading problem", provide more
details so that
other new contributors understand and can also help in contributing to the
project in the future.
Chris.
Re: atomicity problem in SecurityUtil.java
Posted by Mark Thomas <ma...@apache.org>.
On 21/07/2012 21:32, Chris R wrote:
> Hi,
>
> I am looking at SecurityUtil.java (
> http://www.docjar.com/html/api/org/apache/catalina/security/SecurityUtil.java.html)
It would be much better to reference the source from the canonical
location (the ASF svn repo) rather than some unverified third-party
location.
> and it seems that there may be a problem there. More specifically, in the
> execute method, getAttribute of Globals.SUBJECT_ATTR is obtained on a
> session
> and then it is set in a non-atomic fashion.
While that statement is true, you have not explained why this is a
problem in this particular case.
The are many examples in the archives of folks reporting a "problem"
based on an analysis of a small section of code without taking into
account how that code is actually used. The Tomcat developers (this one
at least) are more than a little fed up of these reports mainly because
those doing the reporting haven't done the research to figure out if
there is actually an issue or not.
> http://stackoverflow.com/questions/616601/is-httpsession-thread-safe-are-set-get-attribute-thread-safe-operations/616723#616723
> says that the get and set should be synchronized on a session.
No, it doesn't say that. The relevant text here is the specification
text that is quoted in that question.
> Is this going to be a problem?
You tell me.
> Should I file a bug?
How can you possibly file a bug if you don't know whether or not it is a
problem? Bug reports along the lines of "I think there might be a
problem..." are going to get closed very quickly as INVALID. Probably
with a comment to go and do some research to figure out if there is a
problem.
Having looked at this particular code, I see some potential
opportunities for simplifying things but no chance of a threading
problem. If you analysis reaches a different conclusion, please share it.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org