You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by mw...@apache.org on 2002/05/29 07:24:28 UTC

Watchdog/Reconfigurator Nickel Tour

As promised, here is a review of the current Watchdog/Reconfigurator
premise, design, and implementation.  PLEASE feel free to make comments and
suggestions.  Also note that the version of the code at the bottom of this
message that will be placed in cvs will have the correct package/class name
etc.  I will clean it up.

The premise of the "Watchdog" or reconfigurator came from suggestions from
the user list.  It was suggested that the basic watchdog functionality that
is implemented in DOMConfigurator and PropertyConfigurator
configureAndWatch() methods be broken out and configurable on its own.
Wouldn't it be nice if one could define a watchdog in the configuration file
and then not need to use the configureAndWatch() method in custom code at
all?  More discussion led to the suggestion that the source of the new
configuration data should be expanded to cover more than just files.
Through trial and error the following guidelines in the reconfigurator
design have been synthesized:

1) The purpose of the reconfigurator is to primarily "watch" or "monitor" a
"source" for some indication that said "source" has been changed or updated
with new/different configuration data.

2) Once the change in state has been detected, the reconfigurator is
implemented and configured with enough information to reconfigure log4j
using the new/updated configuration data.

3) The reconfigurator implementation does not deal with the specific format
of the configuration data or even the specific reconfiguration of log4j.  It
delegates this responsibility to the Configurator class, where it belongs.
This means that the reconfigurator design should support any Configurator
implementation.  The most the reconfigurator knows about the source of the
new configuration data is how to locate it and hand it off to the
Configurator.

4) The reconfigurator design should provide for better control over when a
reconfigurator is active and allows for reconfigurators to be stopped and
even restarted.

While configurators deal with the source format (like xml or properties),
reconfigurators deal more with the source type.  The reconfigurator is
implemented to monitor a specific type of thing, like a file or url.  So,
while we have a DOMConfigurator, we can have a FileReconfigurator,
URLReconfigurator, or SocketReconfigurator, all of which can use the
DOMConfigurator to process their new/updated configuration data.

Like Configurators, Reconfigurators must implement a basic interface
required for all Reconfigurator implementations.  This is the Watchdog.java
class listed below (remember, I will rename everything before check in).
Some highlights:

- every configurator can have a name (though where that name is used is
still an open issue).
- The configurator class to use for configuration can be specified via a
property setting.
- Reconfigurators can be started and stopped.

So, to implement a basic reconfigurator is fairly simple from the interface
requirements.  To save developers the trouble of reimplementing the wheel,
there is a base, abstract implementation provided, WatchdogBase.java, below.
Highlights:

- Obviously it implements the required interface.  It uses the
Runnable/Thread classes to implement a process that can run independent of
the parent/main process.
- It requires a couple of methods to be implemented by subclasses,
reconfigure() and checkForReconfiguration().  It is expected that these
methods will need to be specific to the type of source the reconfigurator
has been implemented to watch.
- It allows subclasses to override setup() and cleanup() methods to their
own needs.
- It provides useful helper methods to create a configurator instance, and
to handle configuration using either a URL or an InputStream.  It is
expected that most reconfigurators will use one or the other as a source for
new configuration data, but other source types are not precluded.

And that is enough to digest for now.  Please review the enclosed code
below.  Tomorrow I will post a message that goes into details about some
specific implementations to watch files, urls, and sockets.  They are
actually quite trivial given the base class implementation

When reviewing this design and code, it might be useful to keep the
following items in mind:

1) How easy would it be for a developer to pick up the interface and/or base
class and use it to develop their own reconfigurator?  Does the base class
help or hinder in this regard?
2) Knowing that allowing reconfigurators to be created/started within
settings in a configuration file, does this design lend itself to this goal?
Should the current set of active reconfigurators "live" in a known location?
Where would this location be?  How can this be strictly enforced?
3) Is Reconfigurator the right name for this functionality?

All questions, comments, critiques appreciated.

-Mark


Watchdog.java----------------------------------------------

/**
  Defines the basic interface that all Watchdogs must support.

  Watchdogs will "watch" a log4j configuration source, and when new,
  changed configuration data is available, the watchdog will
  initiate a reconfiguration of the log4j settings using the new
  configuration data.

  Several different watchdog classes are implemented, such as
  FileWatchdog (watches a configuration file), HttpWatchdog (watches
  a configuration file at a url location), and SocketWatchdog (watches
  a socket for incoming configuration data).  More types of watchdogs
  can be easily written subclassing  either the WatchdogBase or
  URLWatchdogBase classes.  Please see those classes for more
  information.  Developers can also create their own versions
  implementing the Watchdog interface.

  All watchdogs can have a name and configurator class.  The configurator
  class will be used when reconfiguring using the new data.  All
  watchdogs can be started and stopped.

  @author Mark Womack
*/
public interface Watchdog {

  /**
    For watchdogs that need an interval setting, this is the
    default, 60 seconds. */

  public static final long DEFAULT_INTERVAL = 60000;

  /**
    Returns true if this watchdog is currently running. */
  public
  boolean isRunning();

  /**
     Set the name of this watchdog. The name is used by other
     components to identify this watchdog. */
  public
  void setName(String name);

  /**
     Get the name of this watchdog. The name uniquely identifies the
     watchdog.  */
  public
  String getName();

  /**
    Sets the configurator class used for reconfiguration. */
  public
  void setConfigurator(String configuratorClassName);

  /**
    Gets the configurator class used for reconfiguration. */
  public
  String getConfigurator();

  /**
    Starts this watchdog watching. After calling this method the
    watchdog will be active. */
  public
  void startWatching();

  /**
    Stops this watchdog. After calling this method the
    watchdog will become inactive, but it is not guaranteed
    to be immediately inactive. If threads are involved in the
    implementation, it may take time for them to be interupted and
    exited. */
  public
  void stopWatching();
}


WatchdogBase.java----------------------------------------------

/**
  Abstract base implementation for Watchdogs.  Subclasses
  are expected to implement the reconfigure() and
  checkForReconfiguration() methods.  Subclasses can provide their
  own implementations of the setup() and cleanup() methods as needed.

  Subclasses can use the helper methods implemented by this class:
  getConfiguratorInstance(), reconfigureByURL(), and
  reconfigureByInputStream().

  The behavior of the watchdog is undefined when setting property
  values while the Watchdog is running.  Property values should be
  set up before starting the watchdog and should remain constant while
  running.

  @author Mark Womack
*/
public abstract class WatchdogBase implements Watchdog, Runnable {

  /**
    The thread running this watchdog. */
  protected Thread watchdogThread;

  /**
    True if this watchdog is running. */
  private boolean running = false;

  /**
    The name of this watchdog. */
  protected String name;

  /**
    The class name of the configurator to use when reconfigurating. */
  protected String configuratorClassName;

  /**
    Returns true if this watchdog is currently running. */
  public
  synchronized
  boolean isRunning() {
    return running;
  }

  /**
     Set the name of this watchdog. The name is used by other
     components to identify this watchdog. */
  public
  void setName(String _name) {
    name = _name;
  }

  /**
     Get the name of this watchdog. The name uniquely identifies the
     watchdog.  */
  public
  String getName() {
    return name;
  }

  /**
    Sets the configurator class used for reconfiguration. */
  public
  void setConfigurator(String _configuratorClassName) {
    configuratorClassName = _configuratorClassName;
  }

  /**
    Gets the configurator class used for reconfiguration. */
  public
  String getConfigurator() {
    return configuratorClassName;
  }

  /**
    Starts this watchdog. After calling this method the
    watchdog will be active. */
  public
  synchronized
  void startWatching() {
    // if not already running
    if (!running) {
      // create a thread to run this
      watchdogThread = new Thread(this);

      // setup the thread
      watchdogThread.setName(this.name);
      watchdogThread.setDaemon(true);

      LogLog.debug(this.getName() + " watchdog starting");

      // start the thread
      running = true;
      watchdogThread.start();
    }
  }

  /**
    Stops this watchdog. After calling this method the
    watchdog will become inactive, but it is not guaranteed
    to be immediately inactive. It may take time for the
    thread to be interupted and exited. */
  public
  synchronized
  void stopWatching() {
    // if already running
    if (running) {
      LogLog.debug(this.getName() + " watchdog stopping");

      // indicate we are no longer supposed to run
      running = false;

      // interrupt the thread if it is sleeping
      watchdogThread.interrupt();

      // lose the reference to this thread for gc
      watchdogThread = null;
    }
  }

  /**
    Implementation required by the Runnable interface.  Calls the
    setup() method, then enters a loop that will run until the
    boolean member running becomes false.  The loop calls the
    checkForReconfiguration() method, and if this method returns
    true, the reconfigure() method is called. When this loop is
    exited, the cleanup() method is called before this method
    is exited. */
  public
  void run()

    try {
      // setup before starting loop
      setup();

      LogLog.debug(this.getName() + " watchdog now active");

      // while we are supposed to be running
      while (isRunning()) {

        // check for new reconfiguration data
        if (checkForReconfiguration() && isRunning()) {

          LogLog.debug(this.getName() + " watchdog reconfiguring");

          reconfigure();
        }
      }
    }
    finally

      // cleanup after finishing loop
      cleanup();
    }

    LogLog.debug(this.name + " watchdog now inactive");
  }

  /**
    Called before the watchdog starts the running loop.  Subclasses can
    override to have specific behavior when activating. */
  protected
  void setup() {
    // subclasses can override to setup before starting main run loop
  }

  /**
    Called after the watchdog stops the running loop.  Subclasses can
    override to have specific behavior when deactivating. */
  protected
  void cleanup() {
    // subclasses can override to cleanup after ending main run loop
  }

  /**
    Called when the watchdog should reconfigure log4j. Subclasses must
    implement, reconfiguring in a way specific to the data source it is
    implemented to watch. */
  abstract
  protected
  void reconfigure();

   /**
    Returns true if watchdog should reconfigure.  Subclasses must implement,
    checking for new, changed configuration data in a way specific to the
    data source it is implemented to watch. */
  abstract
  protected
  boolean checkForReconfiguration();

  /**
    Helper method to get an instance of the configurator class. */
  protected
  Configurator getConfiguratorInstance() {
    // create an instance of the configurator class
    Configurator configurator = null;

    // if we were configured with a configurator class name, use it
    if (configuratorClassName != null) {
      configurator = (Configurator) OptionConverter.instantiateByClassName(
      	configuratorClassName, Configurator.class, null);
    }
    // otherwise, default to PropertyConfigurator
    else {
      configurator = new PropertyConfigurator();
    }

    return configurator;
  }

  /**
    Helper method to reconfigure using a URL.
    The input parameter, configurationURL, should be a URL pointing to
    the configuration data in a format expected by the configurator. */
  protected
  void reconfigureByURL(URL configURL) {
    LogLog.debug(this.getName() + " watchdog reconfiguring using url " +
      configURL);

    // create an instance of the configurator class
    Configurator configurator = getConfiguratorInstance();

    // if able to create configurator, then reconfigure using input stream
    if (configurator != null) {
      configurator.doConfigure(configURL, LogManager.getLoggerRepository());
    }
	  else {
	    LogLog.error(this.getName() + " watchdog could not create
configurator");
	    LogLog.error(this.getName() + " watchdog ignoring new configuration
settings");
	  }

  }

  /**
    Helper method to reconfigure using an InputStream.
    The input parameter, configurationStream, should be a stream
    of configuration data in a format expected by the configurator.
    For this method to work it required that the configurator class
    support a version of the doConfigure() method that takes an
    InputStream. */
  protected
  void reconfigureByInputStream(InputStream configStream) {
    LogLog.debug(this.getName() + " watchdog reconfiguring using
InputStream.");

    // create an instance of the configurator class
    Configurator configurator = getConfiguratorInstance();

    // if able to create configurator, then reconfigure using input stream
    if (configurator != null) {
      try {
      	// use reflection to find a doConfigure() method that supports an
InputStream
	      Class[] classArray = new Class[2];
	      classArray[0] = InputStream.class;
	      classArray[1] = LoggerRepository.class;
     	      Method configMethod =
              configurator.getClass().getMethod("doConfigure", classArray);

      	// call the method
	      Object[] objArray = new Object[2];
	      objArray[0] = configStream;
	      objArray[1] = LogManager.getLoggerRepository();
	      configMethod.invoke(configurator, objArray);
	    } catch (Exception e) {
	  	  LogLog.error(this.getName() +
                " watchdog error working with configurator", e);
	  	  LogLog.error(this.getName() +
                " watchdog ignoring new configuration settings");
	    }
	  }
	  else {
	    LogLog.error(this.getName() +
            " watchdog could not create configurator");
	    LogLog.error(this.getName() +
            " watchdog ignoring new configuration settings");
	  }
  }
}


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: Watchdog/Reconfigurator Nickel Tour

Posted by Ceki Gülcü <ce...@qos.ch>.
At 22:35 30.05.2002 -0700, mwomack@apache.org wrote:
> > It's a good book by the way. If the Thread.interrupt method gives an
> > Computer Science professor and an expert in the field such hard time, I
> > don't think we should attempt to be "smart". Thread.interrupt is not
> > needed. Nice and simple. End of story.
> >
> > Just remove the "watchdogThread.interrupt();" statement and your code
> > will continue to run just as well.
>
>Hm.  I don't think I have a race condition here.

You are not using any wait or notify calls, so, yes, no race conditions.

 > I am just trying to wake up a sleeping thread so the watchdog can
 > shutdown faster.  If you have set the interval to 60 seconds, it could
 > be up to a minute before the watchdog wakes up and figures out it
 > should shutdown.  How can/should I do this differently?

Yes, it could take 60 seconds for the watchDog thread to exit. So? :-)
You do not have to do anything different, except removing the
"watchdogThread.interrupt()" statement.

>Also, you'll notice that I do not use interrupt() for the SocketWatchdog.
>Experience has shown that the interrupt() call does NOT cause the
>ServerSocket.accept() to interrupt itself (contrary to the first link you
>sent).  At least not in JDK 1.3.

As I said in a previous note, interrupt() never does anything useful
except causing confusion. So interrupt() not interrupting
socket.accept() is not surprising.

Does anyone know of a good article explaining why the interrupt()
method should be shunned?

>That is why I set the SoTimeout property
>to 2 seconds.  So, it could take up to 2 seconds for a SocketWatchdog to
>shutdown completely.  That could be a problem if as part of watchdogs being
>allowed in configuration files, the new watchdog being configured wants to
>use the same port number.  It could be up to 2 seconds before the port is
>free from the original watchdog.  Maybe watchdogs should not return from the
>stopWatchdog() method until the watchdog is completely shutdown.

One strategy for dealing with lingering ports is to try again. Try to open 
a port,
if that fails, sleep for 30 seconds and try again. Give up if  the second 
try also fails.

>thanks,
>-Mark

--
Ceki

SUICIDE BOMBING - A CRIME AGAINST HUMANITY
Sign the petition: http://www.petitiononline.com/1234567b
I am signatory number 22106. What is your number?


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: Watchdog/Reconfigurator Nickel Tour

Posted by Ceki Gülcü <ce...@qos.ch>.
At 20:31 30.05.2002 -0700, mwomack@apache.org wrote:
>Ceki,
>
>Thanks for your comments, my comments are below.
>
> > How about "Scout" or "ScriptScout"? A ScriptScout scouts for changes
> > in configuration scripts. Scout is both a noun and a verb, so method
> > names like startScouting or stopScouting make sense.
> >
> > I propose ScriptScout as the interface name and ScriptScoutBase as the
> > base class implemented by FileScout, HTTPScout, SocketScout
> > subclasses.
>
>Hmm.  Well it does make sense.  How about ConfigScout, etc?  I don't know if
>I want to use the word "script"...

Script is shorter than "Configuration File". I use the former as a synonym
for the latter, especially in cases where length matters. ConfigScout is
obviously fine too.

> > Just remove the "watchdogThread.interrupt();" statement and your code
> > will continue to run just as well.
>
>I will look at the references you have included.  I guess I am a little
>concerned about sleeping threads not being woken up to exit the
>reconfigurator sooner.

I see. A sleeping thread does not consume any CPU resources. By
setting the variable "running" cleaner to false, you cause the
watchdogThread to exit peacefully.  The thread will hold to some
memory while sleeping but that is hardly matter for concern. By setting
the watchdogThread as a daemon thread, you have taken care of delays
when exiting the application.

One the other hand by calling watchdogThread.interrupt you must worry
about catching/reacting to InterruptedExceptions and there is no way
to know where or when these will be thrown.  This particular case
might be tractable but why take any risks with no gain to show for it?

>-Mark

--
Ceki

SUICIDE BOMBING - A CRIME AGAINST HUMANITY
Sign the petition: http://www.petitiononline.com/1234567b
I am signatory number 22106. What is your number?


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: Watchdog/Reconfigurator Nickel Tour

Posted by mw...@apache.org.
Ceki,

Thanks for your comments, my comments are below.

> How about "Scout" or "ScriptScout"? A ScriptScout scouts for changes
> in configuration scripts. Scout is both a noun and a verb, so method
> names like startScouting or stopScouting make sense.
>
> I propose ScriptScout as the interface name and ScriptScoutBase as the
> base class implemented by FileScout, HTTPScout, SocketScout
> subclasses.

Hmm.  Well it does make sense.  How about ConfigScout, etc?  I don't know if
I want to use the word "script"...

> The Thread.interrupt() method is evil. It is also absolutely useless
> except for indulging in excruciatingly complex interrupt/notify race
> conditions. Thread.interrupt() makes it unnecessarily complicated to
> write robust multi-threaded code.  As you might except, log4j does not
> contain any Thread.interrupt() method calls.
>

[snip]

> Just remove the "watchdogThread.interrupt();" statement and your code
> will continue to run just as well.

I will look at the references you have included.  I guess I am a little
concerned about sleeping threads not being woken up to exit the
reconfigurator sooner.

> You might want to modify the loop as follows:
>
>    while(isRunning()) {
>      if(reconfigurationNeeded()) {
>         reconfigure();
>      }
>      delay();
>    }

That is a good suggestion.  As you will see in the next posting, the current
design lets the reconfigurator apply the delay in the
checkForReconfiguration() method.  But I think an overridable method is a
better design.

I'll post the other 2 1/2 cents worth of the tour later tonight.

thanks,
-Mark


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: Watchdog/Reconfigurator Nickel Tour

Posted by mw...@apache.org.
> It's a good book by the way. If the Thread.interrupt method gives an
> Computer Science professor and an expert in the field such hard time, I
> don't think we should attempt to be "smart". Thread.interrupt is not
> needed. Nice and simple. End of story.
>
> Just remove the "watchdogThread.interrupt();" statement and your code
> will continue to run just as well.

Hm.  I don't think I have a race condition here.  I am just trying to wake
up a sleeping thread so the watchdog can shutdown faster.  If you have set
the interval to 60 seconds, it could be up to a minute before the watchdog
wakes up and figures out it should shutdown.  How can/should I do this
differently?

Also, you'll notice that I do not use interrupt() for the SocketWatchdog.
Experience has shown that the interrupt() call does NOT cause the
ServerSocket.accept() to interrupt itself (contrary to the first link you
sent).  At least not in JDK 1.3.  That is why I set the SoTimeout property
to 2 seconds.  So, it could take up to 2 seconds for a SocketWatchdog to
shutdown completely.  That could be a problem if as part of watchdogs being
allowed in configuration files, the new watchdog being configured wants to
use the same port number.  It could be up to 2 seconds before the port is
free from the original watchdog.  Maybe watchdogs should not return from the
stopWatchdog() method until the watchdog is completely shutdown.

thanks,
-Mark


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: Watchdog/Reconfigurator Nickel Tour

Posted by Ceki Gülcü <ce...@qos.ch>.
Mark,

Thank you for taking the lead on this. Comments in line.

At 22:24 28.05.2002 -0700, mwomack@apache.org wrote:

>1) The purpose of the reconfigurator is to primarily "watch" or "monitor" a
>"source" for some indication that said "source" has been changed or updated
>with new/different configuration data.

>2) Once the change in state has been detected, the reconfigurator is
>implemented and configured with enough information to reconfigure log4j
>using the new/updated configuration data.

>3) The reconfigurator implementation does not deal with the specific format
>of the configuration data or even the specific reconfiguration of log4j.  It
>delegates this responsibility to the Configurator class, where it belongs.
>This means that the reconfigurator design should support any Configurator
>implementation.  The most the reconfigurator knows about the source of the
>new configuration data is how to locate it and hand it off to the
>Configurator.
>
>4) The reconfigurator design should provide for better control over when a
>reconfigurator is active and allows for reconfigurators to be stopped and
>even restarted.

Yes on all 4 accounts. :-)

[snip]

>When reviewing this design and code, it might be useful to keep the
>following items in mind:
>
>1) How easy would it be for a developer to pick up the interface and/or base
>class and use it to develop their own reconfigurator?  Does the base class
>help or hinder in this regard?
>2) Knowing that allowing reconfigurators to be created/started within
>settings in a configuration file, does this design lend itself to this goal?
>Should the current set of active reconfigurators "live" in a known location?
>Where would this location be?  How can this be strictly enforced?
>3) Is Reconfigurator the right name for this functionality?

How about "Scout" or "ScriptScout"? A ScriptScout scouts for changes
in configuration scripts. Scout is both a noun and a verb, so method
names like startScouting or stopScouting make sense.

I propose ScriptScout as the interface name and ScriptScoutBase as the
base class implemented by FileScout, HTTPScout, SocketScout
subclasses.

>Watchdog.java----------------------------------------------

[snip]

No comments on the interface. It looks complete and perfectly reasonable.


>WatchdogBase.java----------------------------------------------
>
>/**

[snip]

>   @author Mark Womack
>*/
>public abstract class WatchdogBase implements Watchdog, Runnable {
>
>   /**
>     The thread running this watchdog. */
>   protected Thread watchdogThread;
>
>   /**
>     True if this watchdog is running. */
>   private boolean running = false;
>
>   /**
>     The name of this watchdog. */
>   protected String name;
>
>   /**
>     The class name of the configurator to use when reconfigurating. */
>   protected String configuratorClassName;
>
>   /**
>     Returns true if this watchdog is currently running. */
>   public
>   synchronized
>   boolean isRunning() {
>     return running;
>   }
>
>   /**
>      Set the name of this watchdog. The name is used by other
>      components to identify this watchdog. */
>   public
>   void setName(String _name) {
>     name = _name;
>   }
>
>   /**
>      Get the name of this watchdog. The name uniquely identifies the
>      watchdog.  */
>   public
>   String getName() {
>     return name;
>   }
>
>   /**
>     Sets the configurator class used for reconfiguration. */
>   public
>   void setConfigurator(String _configuratorClassName) {
>     configuratorClassName = _configuratorClassName;
>   }
>
>   /**
>     Gets the configurator class used for reconfiguration. */
>   public
>   String getConfigurator() {
>     return configuratorClassName;
>   }
>
>   /**
>     Starts this watchdog. After calling this method the
>     watchdog will be active. */
>   public
>   synchronized
>   void startWatching() {
>     // if not already running
>     if (!running) {
>       // create a thread to run this
>       watchdogThread = new Thread(this);
>
>       // setup the thread
>       watchdogThread.setName(this.name);
>       watchdogThread.setDaemon(true);
>
>       LogLog.debug(this.getName() + " watchdog starting");
>
>       // start the thread
>       running = true;
>       watchdogThread.start();
>     }
>   }
>
>   /**
>     Stops this watchdog. After calling this method the
>     watchdog will become inactive, but it is not guaranteed
>     to be immediately inactive. It may take time for the
>     thread to be interupted and exited. */
>   public
>   synchronized
>   void stopWatching() {
>     // if already running
>     if (running) {
>       LogLog.debug(this.getName() + " watchdog stopping");
>
>       // indicate we are no longer supposed to run
>       running = false;
>
>       // interrupt the thread if it is sleeping
>       watchdogThread.interrupt();

The Thread.interrupt() method is evil. It is also absolutely useless
except for indulging in excruciatingly complex interrupt/notify race
conditions. Thread.interrupt() makes it unnecessarily complicated to
write robust multi-threaded code.  As you might except, log4j does not
contain any Thread.interrupt() method calls.

See for example:

http://www.profcon.com/cargill/jgf/9805/threadingIssues.html
http://elvis.rowan.edu/~hartley/SIGCSE/Workshop/raceConditions.html

In his book "Concurrent Programming", Stephen Hartley a Computer
Science professor gives an example of a Semaphore implemented in Java
using monitors. His implementation contains a interrupt/notify related
race condition. It will take him another 4 iterations until he gets it
right (after the book is published) that is until someone finds a bug
in the 4th iteration.

For more info on the book see:

    http://www.mcs.drexel.edu/~shartley/ConcProgJava/

It's a good book by the way. If the Thread.interrupt method gives an
Computer Science professor and an expert in the field such hard time, I
don't think we should attempt to be "smart". Thread.interrupt is not
needed. Nice and simple. End of story.

Just remove the "watchdogThread.interrupt();" statement and your code
will continue to run just as well.

>       // lose the reference to this thread for gc
>       watchdogThread = null;
>     }
>   }
>
>   /**
>     Implementation required by the Runnable interface.  Calls the
>     setup() method, then enters a loop that will run until the
>     boolean member running becomes false.  The loop calls the
>     checkForReconfiguration() method, and if this method returns
>     true, the reconfigure() method is called. When this loop is
>     exited, the cleanup() method is called before this method
>     is exited. */
>   public
>   void run()
>
>     try {
>       // setup before starting loop
>       setup();
>
>       LogLog.debug(this.getName() + " watchdog now active");
>
>       // while we are supposed to be running
>       while (isRunning()) {
>
>         // check for new reconfiguration data
>         if (checkForReconfiguration() && isRunning()) {
>
>           LogLog.debug(this.getName() + " watchdog reconfiguring");
>
>           reconfigure();
>         }
>       }
>     }

You might want to modify the loop as follows:

   while(isRunning()) {
     if(reconfigurationNeeded()) {
        reconfigure();
     }
     delay();
   }

I renamed checkForReconfiguration as reconfigurationNeeded. The
isRunning check in the if statement is correct but not absolutely
required.  The delay method allows a subclass to introduce a delay if
it needs to have a delay, e.g. FileScout, HTTPScout but not
SocketScout.


>     finally
>
>       // cleanup after finishing loop
>       cleanup();
>     }
>
>     LogLog.debug(this.name + " watchdog now inactive");
>   }

[snip]


--
Ceki

SUICIDE BOMBING - A CRIME AGAINST HUMANITY
Sign the petition: http://www.petitiononline.com/1234567b
I am signer number 22106. What is your number?


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>