Mailing-List: contact tomcat-dev-help@jakarta.apache.org; run by ezmlm
Precedence: bulk
Reply-To: tomcat-dev@jakarta.apache.org
Subject: RE: Tomcat 3.3 contextAdmin issues
From: Costin Manolache <cmanolache@yahoo.com>
To: paulo.gaspar@krankikom.de
Cc: tomcat-dev@jakarta.apache.org
In-Reply-To: <NEBBJJIICDPIINBNIJHJIEDOJPAA.paulo.gaspar@krankikom.de>
References: <NEBBJJIICDPIINBNIJHJIEDOJPAA.paulo.gaspar@krankikom.de>
Content-Type: text/plain
Content-Transfer-Encoding: 7bit
Date: 20 Aug 2001 11:57:09 -0700
Message-Id: <998333829.25941.86.camel@manolache.eng.sun.com>
Mime-Version: 1.0

On 20 Aug 2001 02:01:12 +0200, Paulo Gaspar wrote:

> Your explanation sure helps understanding what functionality is intended
> for each tag. I can take a look at that too. It is easier for me to 
> understand the taglibs than the rest of Tomcat.
> =;o)

Well, I hope understanding the rest of tomcat is not that difficult, but
the goal of the tags was to hide tomcat implementation details ( or
tomcat itself ). A "context" tag can have different implementations,
maybe specific to other servlet containers - the admin interface will be
the same, just a different taglib code will be used.

BTW, nothing requires you to use the tags or jsp - but whatever you use,
please keep the implementation behind an interface similar with the
tags. ( i.e. similar name and behaviors ).


> In this case I am talking about the comments in the method
>     org.apache.tomcat.core.ContextManager.shutdown()
> 
> In this method's source code there are 2 blocks of cleanup code that 
> were commented out. The fact that they were not just removed and the 
> nature of a comment:
>   "remove the modules ( XXX do we need that ? )"
> before one of those blocks makes me wonder how sure it is that they
> are correct.

The code that is commented out used to be part of the method, including
the one with XXX comment ( and the answer is - we don't need to remove
the modules ).

The idea is quite simple - shutdown() is symetrical with init(). If you
add any context manually ( for example EmbededTomcat.addContext() ), you
should also remove it when you stop ( if you want to ). If a module is
adding contexts - it should also remove them ( or leave them in and
don't add them back ).

That's probably where the bug is, I need to review ContextXmlReader and
AutoWebApp to make sure they remove the contexts on shutdown.

shutdown() followed by init() should bring you back to the same state as
you were before ( if no configuration change happened ). 

Now, that's the theory - or what seems to be the 'correct' behavior for
the core. 3.3 is mostly about making sure the core behaves in a well
defined way - better modules will follow, and we can fix modules easily
without all the headache of a major release. 

If you have any doubts about the ContextManager behavior - please speak
now, we may still be able to fix it. 

Regarding the module removal - again, whoever adds modules should also
remove them back, shutdown shouldn't mess with that ( since init doesn't
either ).

In future, some modules will be enhanced to deal 'nicely' with restarts,
and I plan to add support module reloading ( via a module, of course
:-). As I mentioned so many times, after 3.3 we shouldn't have to change
anything in the core, so new modules can rely on some known and well
defined behavior ( well defined doesn't mean perfect, but good enough
:-). 


> Specific questions, besides the above "ContextManager.shutdown()" 
> issue:
>  - Why is it possible to add 2 or more contexts with the same name
>    and base path? It is a cleanup issue that this happens with the 
>    "restart.jsp" code, but shouldn't this kind of duplication also 
>    be prevented?

Contexts should be identified by hostname and base path. If we don't
check for that - it's a bug. BTW, the right way to fix the bug is to add
the check - not in the core, but in a module ( that can do other checks
during the addContext/contextInit phases ). Again, this can wait until
after 3.3, I don't think it's a major issue. 

>  - To make a hot restart, it looks like modules should be restarted
>    too. Is this correct?

Modules should deal themself with the init/shutdown events, and restart.
Most existing modules do not need to be reloaded - but that can be done
too. I'm still investigating how to implement module restart, and "hot"
module add/remove. I don't think this can be done in 3.3, but in a set
of modules that can be released after 3.3 ( I have some code, but now I
want to focus on 3.3 and few other important pieces ).


>  - When using "restart.jsp", previously removed contexts (using the
>    admin pages) were not added back. Why?

Bug probably :-) Again, the right behavior is that whoever adds
something should take it back. 

>  - Where are existing contexts detected and loaded? Is it on a 
>    module? And if yes, then which?

Contexts ( and modules ) are added in 2 ways:
- in applications embedding tomcat, by the application via calls to EmbededTomcat 
or ContextManager. 

- in standalone tomcat, by various configuration modules. Right now the list is:
ContextXmlReader
AutoWebApp.

The first will add modules declared in apps-XXX.xml and server.xml ( which 
shouldn't be used for contexts, that's only for backward compat ).

The second deals with webapps-like dirs ( you can define additional
dirs, very usefull for multiple virtual hosts ).

Thats' where you should check what happens on shutdown.

BTW, this review will help a lot - thank you very much. I think it's very
important to make sure the behavior is right in the first place, we can fix
the modules later ( even after beta2, and even after release 
 - I suspect most of the errors are minor and shouldn't brake anything that works )

Costin