> 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();
>  >       }

The "started" flag indicates that the component has been started. It's
related to the thread state since the thread cannot be started if the
started flag is not set to true. The flag is set by the start() and stop()
method. If the component hasn't been started yet, I don't think it should
start the thread (or try to stop it).
Does it make sense (or did I miss something ?) ?

Remy (going back to optimizing the HTTP connector)

Reply via email to