tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mike Anderson" <MMAND...@novell.com>
Subject [PATCH] for reloading syncronization problem (tomcat_32)
Date Wed, 02 May 2001 22:55:03 GMT
In ServletWrapper.handleReload, there is a syncronization problem.  If tomcat us under a load
and a servlet class file or jsp is updated, there is a potential for multiple request to try
and reload the servlet simultaneously and step on each other causing a bad state.  The way
it is currently coded, we've seen cases where when a class is updated, the first thread through
checks to see if a reload needs to take place and starts the destroy.  The next thread comes
in and also thinks it is supposed to do a reload and starts down the same branch.  Meanwhile,
the first thread continues and creates the new loader (loader.reload) and finishes handleReload
(including setting servlet=null and servletClass=null).  The second thread then continues
it's processing, and gets to creates a new loader.  While the second thread is creating the
new loader, the first thread runs the Handler.service method which runs the ServletWrapper.init
method which calls loadServlet where servlet and servletClass are set appropriately, and eventually
calls ServletWrapper.doInit which sets initialized to true.  Then the second thread continues
and nulls out servlet and servletClass, but since initialized is true, it doesn't run the
init method and from the on you get a null pointer exception trying to access that particular
servlet.  

I've even tried just moving the lines that null out servlet and servletClass up to right after
where initialized is set to false, but this causes another problem where the first thread
creates a new loader and calls loadServlet, then the second thread creates a new loader but
the servlet isn't reinitialized (since it doesn't need to be) and so the loader doesn't have
it cached and always reports false when its shouldReload method is called.  This means that
 the servlet never really gets reloaded because the loader doesn't have it cached.

Anyways, attached is a patch that fixes this problem.  I synchronized before I call load.shouldReload
so that only the first thread will actually call loader.reload with subsequent threads getting
a false back from the new loader so that they don't also try and create a new loader or null
out the class variables.  I've tested this on Windoze (Win2K) and NetWare against the Tomcat
HTTP handler and via Apache/Tomcat.  It does slow performance during the actually reload,
but is minimal otherwise and if the context is set to not reload, the syncronized call is
bypassed altogether.  I would like to get this in to 3.2.2 if at all possible since it does
affect us heavily in our development and testing on NetWare.  Even though the diff seems large,
all that was added was the synchronized call (and it's preceding comment).  The rest of the
differences are just indenting the existing code.



Mike Anderson
Senior Software Engineer
Platform Services Group
mmanders@novell.com
Novell, Inc., the leading provider of Net services software
www.novell.com


Mime
View raw message