Added to the Javadoc

git-svn-id: https://svn.apache.org/repos/asf/incubator/qpid/trunk/qpid@542476 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 156 insertions, 128 deletions

diff --git a/java/broker/src/main/java/org/apache/qpid/server/Main.java b/java/broker/src/main/java/org/apache/qpid/server/Main.java
index c345b43aeb..6f970730ec 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/Main.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/Main.java
@@ -26,8 +26,6 @@ import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.util.Collection;
 import java.util.List;
-import java.util.StringTokenizer;
-
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.HelpFormatter;
 import org.apache.commons.cli.Option;
@@ -36,17 +34,14 @@ import org.apache.commons.cli.Options;
 import org.apache.commons.cli.ParseException;
 import org.apache.commons.cli.PosixParser;
 import org.apache.commons.configuration.ConfigurationException;
-
 import org.apache.log4j.BasicConfigurator;
 import org.apache.log4j.Logger;
 import org.apache.log4j.xml.DOMConfigurator;
-
 import org.apache.mina.common.ByteBuffer;
 import org.apache.mina.common.IoAcceptor;
 import org.apache.mina.common.SimpleByteBufferAllocator;
 import org.apache.mina.transport.socket.nio.SocketAcceptorConfig;
 import org.apache.mina.transport.socket.nio.SocketSessionConfig;
-
 import org.apache.qpid.AMQException;
 import org.apache.qpid.common.QpidProperties;
 import org.apache.qpid.framing.ProtocolVersion;
@@ -66,7 +61,10 @@ import org.apache.qpid.url.URLSyntaxException;
 @SuppressWarnings({ "AccessStaticViaInstance" })
 public class Main
 {
+    /** Used for debugging. */
     private static final Logger _logger = Logger.getLogger(Main.class);
+
+    /** Used for logging operator messages. */
     public static final Logger _brokerLogger = Logger.getLogger("Qpid.Broker");
 
     private static final String DEFAULT_CONFIG_FILE = "etc/config.xml";
@@ -74,8 +72,8 @@ public class Main
     private static final String DEFAULT_LOG_CONFIG_FILENAME = "log4j.xml";
     public static final String QPID_HOME = "QPID_HOME";
     private static final int IPV4_ADDRESS_LENGTH = 4;
-    
-    private static final char IPV4_LITERAL_SEPARATOR = '.';    
+
+    private static final char IPV4_LITERAL_SEPARATOR = '.';
 
     protected static class InitException extends Exception
     {
@@ -88,6 +86,9 @@ public class Main
     protected final Options options = new Options();
     protected CommandLine commandLine;
 
+    /**
+     * @todo Side-effecting constructor; eliminate. Put the processing sequence in the main method.
+     */
     protected Main(String[] args)
     {
         setOptions(options);
@@ -115,30 +116,35 @@ public class Main
         }
     }
 
+    /**
+     * Sets up the command line options, with usage help, ready for parsing the command line against.
+     *
+     * @param options The object to store the configured command line options in.
+     */
     protected void setOptions(Options options)
     {
         Option help = new Option("h", "help", false, "print this message");
         Option version = new Option("v", "version", false, "print the version information and exit");
         Option configFile =
-                OptionBuilder.withArgName("file").hasArg().withDescription("use given configuration file").withLongOpt("config")
-                        .create("c");
+            OptionBuilder.withArgName("file").hasArg().withDescription("use given configuration file").withLongOpt("config")
+                         .create("c");
         Option port =
-                OptionBuilder.withArgName("port").hasArg()
-                        .withDescription("listen on the specified port. Overrides any value in the config file")
-                        .withLongOpt("port").create("p");
+            OptionBuilder.withArgName("port").hasArg()
+                         .withDescription("listen on the specified port. Overrides any value in the config file")
+                         .withLongOpt("port").create("p");
         Option bind =
-                OptionBuilder.withArgName("bind").hasArg()
-                        .withDescription("bind to the specified address. Overrides any value in the config file")
-                        .withLongOpt("bind").create("b");
+            OptionBuilder.withArgName("bind").hasArg()
+                         .withDescription("bind to the specified address. Overrides any value in the config file")
+                         .withLongOpt("bind").create("b");
         Option logconfig =
-                OptionBuilder.withArgName("logconfig").hasArg()
-                        .withDescription("use the specified log4j xml configuration file. By "
-                                         + "default looks for a file named " + DEFAULT_LOG_CONFIG_FILENAME
-                                         + " in the same directory as the configuration file").withLongOpt("logconfig").create("l");
+            OptionBuilder.withArgName("logconfig").hasArg()
+                         .withDescription("use the specified log4j xml configuration file. By "
+                + "default looks for a file named " + DEFAULT_LOG_CONFIG_FILENAME
+                + " in the same directory as the configuration file").withLongOpt("logconfig").create("l");
         Option logwatchconfig =
-                OptionBuilder.withArgName("logwatch").hasArg()
-                        .withDescription("monitor the log file configuration file for changes. Units are seconds. "
-                                         + "Zero means do not check for changes.").withLongOpt("logwatch").create("w");
+            OptionBuilder.withArgName("logwatch").hasArg()
+                         .withDescription("monitor the log file configuration file for changes. Units are seconds. "
+                + "Zero means do not check for changes.").withLongOpt("logwatch").create("w");
 
         options.addOption(help);
         options.addOption(version);
@@ -149,6 +155,11 @@ public class Main
         options.addOption(bind);
     }
 
+    /**
+     * @todo Handles command line, but there is already a parse command line method. Put all command line handling
+     *       in a single flow of control. Also part implements the top-level handler, which would more neatly be kept
+     *       together in one place.
+     */
     protected void execute()
     {
         // note this understands either --help or -h. If an option only has a long name you can use that but if
@@ -204,6 +215,14 @@ public class Main
         }
     }
 
+    /**
+     * Reads the configuration file and performs configuration specified in it. Then hands over to bind to do the
+     * actual broker start-up.
+     *
+     * @todo A bit confusing, seperate out configuration from start-up. Call config method to handle the configuration
+     *       from the config file, in the main flow of control (possibly #main method). Then call #startup or #bind
+     *       to start the broker.
+     */
     protected void startup() throws InitException, ConfigurationException, Exception
     {
         final String QpidHome = System.getProperty(QPID_HOME);
@@ -241,14 +260,14 @@ public class Main
 
         ApplicationRegistry.initialise(new ConfigurationFileApplicationRegistry(configFile));
 
-        //fixme .. use QpidProperties.getVersionString when we have fixed the classpath issues
+        // fixme .. use QpidProperties.getVersionString when we have fixed the classpath issues
         // that are causing the broker build to pick up the wrong properties file and hence say
-        // Starting Qpid Client 
-        _brokerLogger.info("Starting Qpid Broker " + QpidProperties.getReleaseVersion()
-                           + " build: " + QpidProperties.getBuildVersion());
+        // Starting Qpid Client
+        _brokerLogger.info("Starting Qpid Broker " + QpidProperties.getReleaseVersion() + " build: "
+            + QpidProperties.getBuildVersion());
 
         ConnectorConfiguration connectorConfig =
-                ApplicationRegistry.getInstance().getConfiguredObject(ConnectorConfiguration.class);
+            ApplicationRegistry.getInstance().getConfiguredObject(ConnectorConfiguration.class);
 
         ByteBuffer.setUseDirectBuffers(connectorConfig.enableDirectBuffers);
 
@@ -295,11 +314,10 @@ public class Main
         }
 
         bind(port, connectorConfig);
-
     }
 
     protected void setupVirtualHosts(String configFileParent, String configFilePath)
-            throws ConfigurationException, AMQException, URLSyntaxException
+        throws ConfigurationException, AMQException, URLSyntaxException
     {
         String configVar = "${conf}";
 
@@ -326,13 +344,19 @@ public class Main
                 if (fileNames[each].endsWith(".xml"))
                 {
                     VirtualHostConfiguration vHostConfig =
-                            new VirtualHostConfiguration(configFilePath + "/" + fileNames[each]);
+                        new VirtualHostConfiguration(configFilePath + "/" + fileNames[each]);
                     vHostConfig.performBindings();
                 }
             }
         }
     }
 
+    /**
+     * Assembles/configures the components that Mina needs, then start Mina running to accept connections.
+     *
+     * @todo Partially implements top-level error handler. Better to let these errors fall through to a single
+     *       top-level handler.
+     */
     protected void bind(int port, ConnectorConfiguration connectorConfig)
     {
         String bindAddr = commandLine.getOptionValue("b");
@@ -373,7 +397,7 @@ public class Main
                 }
 
                 acceptor.bind(bindAddress, handler, sconfig);
-                //fixme  qpid.AMQP should be using qpidproperties to get value
+                // fixme  qpid.AMQP should be using qpidproperties to get value
                 _brokerLogger.info("Qpid.AMQP listening on non-SSL address " + bindAddress);
             }
 
@@ -383,8 +407,8 @@ public class Main
                 try
                 {
 
-                   acceptor.bind(new InetSocketAddress(connectorConfig.sslPort), handler, sconfig);
-                    //fixme  qpid.AMQP should be using qpidproperties to get value
+                    acceptor.bind(new InetSocketAddress(connectorConfig.sslPort), handler, sconfig);
+                    // fixme  qpid.AMQP should be using qpidproperties to get value
                     _brokerLogger.info("Qpid.AMQP listening on SSL port " + connectorConfig.sslPort);
 
                 }
@@ -394,9 +418,9 @@ public class Main
                 }
             }
 
-            //fixme  qpid.AMQP should be using qpidproperties to get value            
-            _brokerLogger.info("Qpid Broker Ready :" + QpidProperties.getReleaseVersion()
-                               + " build: " + QpidProperties.getBuildVersion());
+            // fixme  qpid.AMQP should be using qpidproperties to get value
+            _brokerLogger.info("Qpid Broker Ready :" + QpidProperties.getReleaseVersion() + " build: "
+                + QpidProperties.getBuildVersion());
         }
         catch (Exception e)
         {
@@ -404,38 +428,60 @@ public class Main
         }
     }
 
+    /**
+     * Processes the command line and starts the broker running. This method acts as a top-level error handler for
+     * any exceptions that fall out of the code below this point. These exceptions are logged before System.exit is
+     * called with an error code.
+     *
+     * @param args The command line arguments.
+     */
     public static void main(String[] args)
     {
+        // Use a try block so that any exceptions that fall through to the top-level are logged before the application
+        // exits with an error code.
+        // try
+        {
+            // Parse the command line.
+
+            // Create an instance of the Main broker entry point class and start it running.
+        }
+        // catch ()
+        {
+            // Log the exception as an error.
+
+            // Exit with an error code.
+        }
 
         new Main(args);
     }
 
     private byte[] parseIP(String address) throws Exception
     {
-        char[] literalBuffer = address.toCharArray();    
+        char[] literalBuffer = address.toCharArray();
         int byteCount = 0;
         int currByte = 0;
         byte[] ip = new byte[IPV4_ADDRESS_LENGTH];
-        for (int i = 0 ; i < literalBuffer.length ; i++) 
+        for (int i = 0; i < literalBuffer.length; i++)
         {
             char currChar = literalBuffer[i];
-            if ((currChar >= '0') && (currChar <= '9')) 
+            if ((currChar >= '0') && (currChar <= '9'))
             {
-            	currByte = (currByte * 10) + (Character.digit(currChar, 10) & 0xFF);
-            } 
+                currByte = (currByte * 10) + (Character.digit(currChar, 10) & 0xFF);
+            }
 
-            if (currChar == IPV4_LITERAL_SEPARATOR || (i + 1 == literalBuffer.length)) 
+            if ((currChar == IPV4_LITERAL_SEPARATOR) || ((i + 1) == literalBuffer.length))
             {
-                ip[byteCount++] = (byte)currByte;
+                ip[byteCount++] = (byte) currByte;
                 currByte = 0;
-            } 
+            }
         }
 
         if (byteCount != 4)
         {
             throw new Exception("Invalid IP address: " + address);
-        }     
-        return ip;        
+        }
+
+        return ip;
     }
 
     private void configureLogging(File logConfigFile, String logWatchConfig)
@@ -448,7 +494,7 @@ public class Main
         catch (NumberFormatException e)
         {
             System.err.println("Log watch configuration value of " + logWatchConfig + " is invalid. Must be "
-                               + "a non-negative integer. Using default of zero (no watching configured");
+                + "a non-negative integer. Using default of zero (no watching configured");
         }
 
         if (logConfigFile.exists() && logConfigFile.canRead())
@@ -457,7 +503,7 @@ public class Main
             if (logWatchTime > 0)
             {
                 System.out.println("log file " + logConfigFile.getAbsolutePath() + " will be checked for changes every "
-                                   + logWatchTime + " seconds");
+                    + logWatchTime + " seconds");
                 // log4j expects the watch interval in milliseconds
                 DOMConfigurator.configureAndWatch(logConfigFile.getAbsolutePath(), logWatchTime * 1000);
             }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQPFastProtocolHandler.java b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQPFastProtocolHandler.java
index d8b7814d31..84ec91d569 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQPFastProtocolHandler.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/protocol/AMQPFastProtocolHandler.java
@@ -7,9 +7,9 @@
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -22,7 +22,6 @@ package org.apache.qpid.server.protocol;
 
 import java.io.IOException;
 import java.net.InetSocketAddress;
-
 import org.apache.log4j.Logger;
 import org.apache.mina.common.ByteBuffer;
 import org.apache.mina.common.IdleStatus;
@@ -31,7 +30,6 @@ import org.apache.mina.common.IoSession;
 import org.apache.mina.filter.SSLFilter;
 import org.apache.mina.filter.codec.ProtocolCodecFilter;
 import org.apache.mina.util.SessionUtil;
-
 import org.apache.qpid.AMQException;
 import org.apache.qpid.codec.AMQCodecFactory;
 import org.apache.qpid.framing.AMQDataBlock;
@@ -60,10 +58,9 @@ public class AMQPFastProtocolHandler extends IoHandlerAdapter
 
     private final IApplicationRegistry _applicationRegistry;
 
-
     public AMQPFastProtocolHandler(Integer applicationRegistryInstance)
     {
-    	this(ApplicationRegistry.getInstance(applicationRegistryInstance));
+        this(ApplicationRegistry.getInstance(applicationRegistryInstance));
     }
 
     public AMQPFastProtocolHandler(IApplicationRegistry applicationRegistry)
@@ -87,41 +84,43 @@ public class AMQPFastProtocolHandler extends IoHandlerAdapter
 
         final ProtocolCodecFilter pcf = new ProtocolCodecFilter(codecFactory);
 
-        ConnectorConfiguration connectorConfig = ApplicationRegistry.getInstance().
-                getConfiguredObject(ConnectorConfiguration.class);
+        ConnectorConfiguration connectorConfig =
+            ApplicationRegistry.getInstance().getConfiguredObject(ConnectorConfiguration.class);
         if (connectorConfig.enableExecutorPool)
         {
             if (connectorConfig.enableSSL && isSSLClient(connectorConfig, protocolSession))
             {
-            	String keystorePath = connectorConfig.keystorePath;
-            	String keystorePassword = connectorConfig.keystorePassword;
-            	String certType = connectorConfig.certType;
-            	SSLContextFactory sslContextFactory = new SSLContextFactory(keystorePath, keystorePassword, certType);
+                String keystorePath = connectorConfig.keystorePath;
+                String keystorePassword = connectorConfig.keystorePassword;
+                String certType = connectorConfig.certType;
+                SSLContextFactory sslContextFactory = new SSLContextFactory(keystorePath, keystorePassword, certType);
                 protocolSession.getFilterChain().addAfter("AsynchronousReadFilter", "sslFilter",
-                                                          new SSLFilter(sslContextFactory.buildServerContext()));
+                    new SSLFilter(sslContextFactory.buildServerContext()));
             }
+
             protocolSession.getFilterChain().addBefore("AsynchronousWriteFilter", "protocolFilter", pcf);
         }
         else
         {
-        	protocolSession.getFilterChain().addLast("protocolFilter", pcf);
+            protocolSession.getFilterChain().addLast("protocolFilter", pcf);
             if (connectorConfig.enableSSL && isSSLClient(connectorConfig, protocolSession))
             {
-            	String keystorePath = connectorConfig.keystorePath;
-            	String keystorePassword = connectorConfig.keystorePassword;
-            	String certType = connectorConfig.certType;
-            	SSLContextFactory sslContextFactory = new SSLContextFactory(keystorePath, keystorePassword, certType);
+                String keystorePath = connectorConfig.keystorePath;
+                String keystorePassword = connectorConfig.keystorePassword;
+                String certType = connectorConfig.certType;
+                SSLContextFactory sslContextFactory = new SSLContextFactory(keystorePath, keystorePassword, certType);
                 protocolSession.getFilterChain().addBefore("protocolFilter", "sslFilter",
-                                                          new SSLFilter(sslContextFactory.buildServerContext()));
-            }        	
-            
+                    new SSLFilter(sslContextFactory.buildServerContext()));
+            }
+
         }
     }
 
     /**
      * Separated into its own, protected, method to allow easier reuse
      */
-    protected void createSession(IoSession session, IApplicationRegistry applicationRegistry, AMQCodecFactory codec) throws AMQException
+    protected void createSession(IoSession session, IApplicationRegistry applicationRegistry, AMQCodecFactory codec)
+        throws AMQException
     {
         new AMQMinaProtocolSession(session, applicationRegistry.getVirtualHostRegistry(), codec);
     }
@@ -135,8 +134,8 @@ public class AMQPFastProtocolHandler extends IoHandlerAdapter
     {
         _logger.info("Protocol Session closed");
         final AMQProtocolSession amqProtocolSession = AMQMinaProtocolSession.getAMQProtocolSession(protocolSession);
-        //fixme  -- this can be null
-        if(amqProtocolSession != null)
+        // fixme  -- this can be null
+        if (amqProtocolSession != null)
         {
             amqProtocolSession.closeSession();
         }
@@ -145,14 +144,14 @@ public class AMQPFastProtocolHandler extends IoHandlerAdapter
     public void sessionIdle(IoSession session, IdleStatus status) throws Exception
     {
         _logger.debug("Protocol Session [" + this + "] idle: " + status);
-        if(IdleStatus.WRITER_IDLE.equals(status))
+        if (IdleStatus.WRITER_IDLE.equals(status))
         {
-            //write heartbeat frame:
+            // write heartbeat frame:
             session.write(HeartbeatBody.FRAME);
         }
-        else if(IdleStatus.READER_IDLE.equals(status))
+        else if (IdleStatus.READER_IDLE.equals(status))
         {
-            //failover:
+            // failover:
             throw new IOException("Timed out while waiting for heartbeat from peer.");
         }
 
@@ -170,22 +169,21 @@ public class AMQPFastProtocolHandler extends IoHandlerAdapter
 
             _logger.error("Error in protocol initiation " + session + ": " + throwable.getMessage(), throwable);
         }
-        else if(throwable instanceof IOException)
+        else if (throwable instanceof IOException)
         {
             _logger.error("IOException caught in" + session + ", session closed implictly: " + throwable, throwable);
         }
         else
         {
             _logger.error("Exception caught in" + session + ", closing session explictly: " + throwable, throwable);
-            
+
             // Be aware of possible changes to parameter order as versions change.
-            protocolSession.write(ConnectionCloseBody.createAMQFrame(0,
-            	session.getProtocolMajorVersion(),
-                session.getProtocolMinorVersion(),	// AMQP version (major, minor)
-            	0,	// classId
-                0,	// methodId
-                200,	// replyCode
-                new AMQShortString(throwable.getMessage())	// replyText
+            protocolSession.write(ConnectionCloseBody.createAMQFrame(0, session.getProtocolMajorVersion(),
+                    session.getProtocolMinorVersion(), // AMQP version (major, minor)
+                    0, // classId
+                    0, // methodId
+                    200, // replyCode
+                    new AMQShortString(throwable.getMessage()) // replyText
                 ));
             protocolSession.close();
         }
@@ -212,7 +210,8 @@ public class AMQPFastProtocolHandler extends IoHandlerAdapter
         }
         else
         {
-            throw new IllegalStateException("Handed unhandled message. message.class = " + message.getClass() + " message = " + message);
+            throw new IllegalStateException("Handed unhandled message. message.class = " + message.getClass() + " message = "
+                + message);
         }
     }
 
@@ -230,11 +229,11 @@ public class AMQPFastProtocolHandler extends IoHandlerAdapter
             _logger.debug("Message sent: " + object);
         }
     }
-    
-    protected boolean isSSLClient(ConnectorConfiguration connectionConfig,
-    		IoSession protocolSession) 
+
+    protected boolean isSSLClient(ConnectorConfiguration connectionConfig, IoSession protocolSession)
     {
-    	InetSocketAddress addr = (InetSocketAddress) protocolSession.getLocalAddress();
-    	return addr.getPort() == connectionConfig.sslPort;
+        InetSocketAddress addr = (InetSocketAddress) protocolSession.getLocalAddress();
+
+        return addr.getPort() == connectionConfig.sslPort;
     }
 }
diff --git a/java/client/src/main/java/org/apache/qpid/client/failover/FailoverException.java b/java/client/src/main/java/org/apache/qpid/client/failover/FailoverException.java
index 49377fdc19..95899d533a 100644
--- a/java/client/src/main/java/org/apache/qpid/client/failover/FailoverException.java
+++ b/java/client/src/main/java/org/apache/qpid/client/failover/FailoverException.java
@@ -7,9 +7,9 @@
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
- * 
+ *
  *   http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing,
  * software distributed under the License is distributed on an
  * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -21,8 +21,12 @@
 package org.apache.qpid.client.failover;
 
 /**
- * This exception is thrown when failover is taking place and we need to let other
- * parts of the client know about this.
+ * This exception is thrown when failover is taking place and otherparts of the client need to know about this.
+ *
+ * @todo This exception is created and passed as an argument to a method, rather than thrown. The exception is being
+ *       used to represent a signal, passed out to other threads. Use of exceptions as arguments rather than as
+ *       exceptions is extremly confusing. Eliminate. Use a Condition or set a flag and check it instead. Also
+ *       FailoverException is Runtime but handled and should only use Runtimes for non-handleable conditions.
  */
 public class FailoverException extends RuntimeException
 {
diff --git a/java/common/src/main/java/org/apache/qpid/pool/PoolingFilter.java b/java/common/src/main/java/org/apache/qpid/pool/PoolingFilter.java
index ffc8f1643a..7f6b1d40ac 100644
--- a/java/common/src/main/java/org/apache/qpid/pool/PoolingFilter.java
+++ b/java/common/src/main/java/org/apache/qpid/pool/PoolingFilter.java
@@ -22,13 +22,10 @@ package org.apache.qpid.pool;
 
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
-
 import org.apache.log4j.Logger;
-
 import org.apache.mina.common.IdleStatus;
 import org.apache.mina.common.IoFilterAdapter;
 import org.apache.mina.common.IoSession;
-
 import org.apache.qpid.pool.Event.CloseEvent;
 
 /**
@@ -201,19 +198,6 @@ public abstract class PoolingFilter extends IoFilterAdapter implements Job.JobCo
         return (Job) session.getAttribute(_name);
     }
 
-    /*private Job createJobForSession(IoSession session)
-    {
-        return addJobForSession(session, new Job(session, this, _maxEvents));
-    }*/
-
-    /*private Job addJobForSession(IoSession session, Job job)
-    {
-        // atomic so ensures all threads agree on the same job
-        Job existing = _jobs.putIfAbsent(session, job);
-
-        return (existing == null) ? job : existing;
-    }*/
-
     /**
      * Implements a terminal continuation for the {@link Job} for this filter. Whenever the Job completes its processing
      * of a batch of events this is called. This method simply re-activates the job, if it has more events to process.
@@ -223,15 +207,6 @@ public abstract class PoolingFilter extends IoFilterAdapter implements Job.JobCo
      */
     public void completed(IoSession session, Job job)
     {
-        // if (job.isComplete())
-        // {
-        // job.release();
-        // if (!job.isReferenced())
-        // {
-        // _jobs.remove(session);
-        // }
-        // }
-        // else
         if (!job.isComplete())
         {
             // ritchiem : 2006-12-13 Do we need to perform the additional checks here?
diff --git a/java/common/src/main/java/org/apache/qpid/util/CommandLineParser.java b/java/common/src/main/java/org/apache/qpid/util/CommandLineParser.java
index 61955160be..9bb4a6635f 100644
--- a/java/common/src/main/java/org/apache/qpid/util/CommandLineParser.java
+++ b/java/common/src/main/java/org/apache/qpid/util/CommandLineParser.java
@@ -603,12 +603,21 @@ public class CommandLineParser
     }

 

     /**

-     * Extracts all name=value pairs from the command line, sets them all as system properties and also returns

-     * a map of properties containing them.

+     * Parses the command line arguments against the specified command line format, printing errors and usage

+     * instrucitons and calling System.exit on errors. Extracts all trailing name=value pairs from the command line,

+     * and sets them all as system properties and also returns a map of properties containing them.

      *

      * @param args The command line.

      *

      * @return A set of properties containing all name=value pairs from the command line.

+     *

+     * @todo Replace the call to System.exit with throwing an exception. Gives caller the option to decide how to

+     *       handle the error.

+     *

+     * @todo Pass in the location to write the usage to as an argument, instead of hard-coding System.out.

+     *

+     * @todo Allow the Properties to add trailing options to be specified as an argument rather than hard coding

+     *       system properties. Again, gives the caller the option to decide.

      */

     public static Properties processCommandLine(String[] args, CommandLineParser commandLine)

     {

diff --git a/java/integrationtests/src/main/java/org/apache/qpid/interop/coordinator/Coordinator.java b/java/integrationtests/src/main/java/org/apache/qpid/interop/coordinator/Coordinator.java
index 2f9937cae9..7c5079327e 100644
--- a/java/integrationtests/src/main/java/org/apache/qpid/interop/coordinator/Coordinator.java
+++ b/java/integrationtests/src/main/java/org/apache/qpid/interop/coordinator/Coordinator.java
@@ -23,15 +23,11 @@ package org.apache.qpid.interop.coordinator;
 import java.io.*;

 import java.util.*;

 import java.util.concurrent.LinkedBlockingQueue;

-

 import javax.jms.*;

-

 import junit.framework.Test;

 import junit.framework.TestResult;

 import junit.framework.TestSuite;

-

 import org.apache.log4j.Logger;

-

 import org.apache.qpid.interop.coordinator.testcases.CoordinatingTestCase1DummyRun;

 import org.apache.qpid.interop.coordinator.testcases.CoordinatingTestCase2BasicP2P;

 import org.apache.qpid.interop.coordinator.testcases.CoordinatingTestCase3BasicPubSub;

@@ -39,7 +35,6 @@ import org.apache.qpid.interop.testclient.TestClient;
 import org.apache.qpid.util.CommandLineParser;

 import org.apache.qpid.util.ConversationFactory;

 import org.apache.qpid.util.PrettyPrintingUtils;

-

 import uk.co.thebadgerset.junit.extensions.TKTestResult;

 import uk.co.thebadgerset.junit.extensions.TKTestRunner;

 import uk.co.thebadgerset.junit.extensions.WrappedSuiteTestDecorator;

@@ -119,7 +114,7 @@ public class Coordinator extends TKTestRunner
         try

         {

             // Use the command line parser to evaluate the command line with standard handling behaviour (print errors

-            // and usage then exist if there are errors).

+            // and usage then exit if there are errors).

             Properties options =

                 CommandLineParser.processCommandLine(args,

                     new CommandLineParser(