Hi Jason,
See below.
Jason Brittain wrote:
> Hi guys!
>
> In reading through the org.apache.catalina.resources package code, I found
> a small omission from the ResourcesBase.setCheckInterval() method:
>
> public void setCheckInterval(int checkInterval) {
>
> // Perform the property update
> int oldCheckInterval = this.checkInterval;
> this.checkInterval = checkInterval;
> support.firePropertyChange("checkInterval",
> new Integer(oldCheckInterval),
> new Integer(this.checkInterval));
>
> // Start or stop the background thread (if necessary)
> if (started) {
> if ((oldCheckInterval > 0) && (this.checkInterval <= 0))
> threadStop();
> else if ((oldCheckInterval <= 0) && (this.checkInterval > 0))
> threadStart();
> }
>
> }
>
> At the end of this method, we changed the check interval, and then we
> need to either start or stop the background thread that periodically
> checks for resource updates. The code in this method handles the
> following:
>
> 1. When the background thread is already running and we should be shutting
> it down because the new check interval doesn't require a background
> thread at all.
> 2. When the thread is supposedly already running, the old check interval
> didn't require a background thread (really, an illegal state -- and
> since
> this code looks basically like my patch below, was this just bad
> placement
> of this code?), and the new check interval does require a background
> thread.. In that case the code at least makes sure that we have
> a reference to a thread.
>
> But, what it doesn't handle is:
>
> 3. When the background thread isn't already running, the previous check
> interval didn't require a background thread, and the new check interval
> does require a background thread.. It should start one.
>
> So, here's the patch I'd suggest:
>
> 280a281,284
> > else {
> > if ((oldCheckInterval <= 0) && (this.checkInterval > 0))
> > threadStart();
> > }
>
You're correct that this kind of code is appropriate (because the component
has already been started without the thread). What's puzzling me is that my
copy of ResourcesBase.java (version 1.3, last updated 2000/10/17) already
includes code very similar to this. Checking the CVS logs, it seems that this
code has been there at least since we moved Catalina over to the new
"jakarta-tomcat-4.0" CVS repository in August.
What version of Tomcat 4.0 sources are you looking at?
>
> Thanks!
>
> --
> Jason Brittain (415)354-6645
Craig
