tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Johnny Kewl" <j...@kewlstuff.co.za>
Subject Re: On Tomcat Concurrency Problems
Date Fri, 16 May 2008 11:00:25 GMT

----- Original Message ----- 
From: "Jim Manico" <jim@manico.net>
To: "Tomcat Developers List" <dev@tomcat.apache.org>
Sent: Friday, May 16, 2008 6:37 AM
Subject: On Tomcat Concurrency Problems

Yes, exactly... which programmer gets every scope and modifier right on a 
"purpose" built module.
I'm absolutely sure the guy is right and they picking up a few niceties 
because TC has now been "absorbed" into their product and its now operating 
in a foreign environment. All of a sudden a valve that was purpose built for 
one function now has to deal with EJB tools trying to use it as well.
Maybe they just shouldnt be using TC like that in the first place, and 
everyone knows that valves are a TC specific things and live outside the 
servlet spec.
I wouldnt worry about it, if you listen very carefully, you can hear glass 
breaking and someone selling a book ;)

I dont care who the programmer is, code goes through interations, and if its 
purpose built for one environment, that never happens, but that also doesnt 
mean its broken, just good for what it was built for.

Maybe now that they have toughened up the code for multipurpose use, they'll 
donate it to Tomcat... what do you think ;)

>A senior developer from the company I work for had the following
> comments (in support of Tomcat) regarding the very interesting threading
> post from Lloyd Chambers last week.
>
> ****
>
> This is an interesting post, but not for the reason that the author
> intended. I'm sure he's an awesome developer who totally understands
> this stuff.  And the findings might be serious problems.  But if they
> are real, *he blew the writeups (to the Tomcat dev list).*
>
> Reread these findings -- they're all theoretical.  I guess they all sort
> of represent "violation of best practice". If you ever write that it
> just means that you can't find an actual problem to report.  Every one
> of these findings raises obvious questions that the finding should have
> answered.  *Always answer "who cares"*
>
> · How is SingleSignOn actually instantiated?  (my guess is that there
> aren't lots of threads at startup).
> · Are the "public" get/setRequireReauthentiation() methods actually
> called from any different threads?  (my guess is that this doesn't make
> a bit of difference).
> · Are there any classes that extend SingleSignOn?  (if not then who
> cares about subclasses modifying non-finals).
> · What actually calls findSessions() and does it modify the array?  (my
> guess is not)
>
> The conclusion that SSO is a "disaster waiting to happen" *is the kind
> of FUD (fear uncertainty doubt) talk that undermines the whole point.*
> The reference to "Java Concurrency in Practice" is probably
> well-intentioned, but just comes off snotty.
>
> We face these dangers in our writings (as an Application Security
> company) as well. Always try to put yourself in the customer's *(in this
> case, the Tomcat dev team) *shoes and really explain more than just the
> narrow-scope technical problem.  If you're using the words this guy is
> using -- something is wrong with your findings:
>
> ·        "Perhaps"
> ·        "Could"
> ·        "Presumably"
> ·        "What if"
> ·        "Bug-prone if"
>
> -- 
> Jim Manico, Senior Application Security Engineer
> jim.manico@aspectsecurity.com | jim@manico.net
> (301) 604-4882 (work)
> (808) 652-3805 (cell)
>
> Aspect Security^(TM)
> Securing your applications at the source
> http://www.aspectsecurity.com
>
>
>
> (post from Lloyd Chambers)
>
> First, I'm an experienced developer, and well-versed in Java threading.
> My main work is on the Glassfish project at Sun.
>
> I've been looking into the code of
> org.apache.catalina.authenticator.SingleSignOn to see how it works, and
> I've noticed a number of thread-safety bugs.  My understanding is that
> Valve SingleSignOn could run on any thread (eg any incoming request).
>
> 1.  Most of the instance variables are not 'final'.  So when
> SingleSignOn is instantiated, there is no guarantee that the instance
> will be seen correctly by any other thread; it's state is undefined from
> the point of view of another thread.
>
> Its immutable variable values (eg the Maps) should be 'final' so as to
> benefit from the JVM guarantee of thread safety for 'final' instance
> variables.  And/or there needs to be an external synchronizing
> side-effect that would make the object instance "visible" to other
> threads.  Perhaps there exists such an external side effect that masks
> this bug.
>
> 2.  The get/set methods are thread-unsafe.  For example,
> getRequireReauthentication() and setRequireReauthentication() are not
> 'synchronized', and the variable 'requireReauthentication' is not
> 'volatile'.  As a result, different threads could see different values
> and/or never see an updated value.  Since these are public methods, and
> presumably can be called from any number of different threads, this is a
> problem.
>
> 3. Variables 'lifecycle' and 'reverse' and 'cache' are not 'final'.  See
> #1 above, but this also exposes/encourages another thread-safety issue:
> what if the variables themselves are changed by a subclass (since
> SingleSignOn is not a 'final' class).
>
> 4.  The Map classes use external synchronization, which is bug-prone if
> any modifications are made (and there might be some race conditions here
> as well).  The java.util.concurrent.ConcurrentHashMap class is much more
> scalable and doesn't have these risks.
>
> 5.  Method deregister() (both variants) contain a race condition whereby
> the call sso.findSessions(). Method findSessions() is 'synchronized',
> but *returns its internal array 'sessions'*!!!  Upon return there is no
> further thread-safety; 'sessions' is now exposed and anything looking at
> it is now subject to a race condition and/or "visibility" problem.
>
> These are almost certainly not all the issues.  From what I can see,
> SingleSignOn is a disaster waiting to happen on the wrong hardware,
> where caches are not write-through as they are on most of today's
> processors.  And even on common hardware (eg Intel), there are still
> some "windows of opportunity" for failures to occur, especially item #5
> above.
>
> The class SingleSignOnEntry is also not thread-safe; for example
> updateCredentials() is thread-unsafe.
>
> I recommend "Java Concurrency in Practice" for understanding these issues.
>
>
> Lloyd Chambers
> http://diglloyd.com </exchweb/bin/redir.asp?URL=http://diglloyd.com/>
>
> [Mac OS X 10.5.2 Intel, Tomcat 6.0.16]
>
---------------------------------------------------------------------------
HARBOR: http://coolharbor.100free.com/index.htm
The most powerful application server on earth.
The only real POJO Application Server.
--------------------------------------------------------------------------- 


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


Mime
View raw message