QPID-504: Print bound port if --port 0 is specified. Not yet used by tests.

	* qpidd.cpp:
	 - With --port 0 print the bound port number to stdout.
	 - Removed --ppid, --check now prints pid.

	* Daemon.cpp/h: Move pid-file generation to caller (qpidd.cpp)

	* Exception.cpp: Log a debug message in exception constructors.
	  Helps to show what exceptions were thrown even if they aren't
	  logged at a higher level. 

	* daemon_test: Test new daemon options.


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

5 files changed, 201 insertions, 148 deletions

diff --git a/cpp/src/qpid/Exception.cpp b/cpp/src/qpid/Exception.cpp
index e73f64f2ba..f4eeb7931b 100644
--- a/cpp/src/qpid/Exception.cpp
+++ b/cpp/src/qpid/Exception.cpp
@@ -19,7 +19,9 @@
  *
  */
 
+#include "qpid/log/Statement.h"
 #include "Exception.h"
+#include <typeinfo>
 #include <errno.h>
 
 namespace qpid {
@@ -28,12 +30,17 @@ std::string strError(int err) {
     char buf[512];
     return std::string(strerror_r(err, buf, sizeof(buf)));
 }
+
+static void ctorLog(const std::exception* e) {
+    QPID_LOG(trace, "Exception constructor " << typeid(e).name() << ": " << e->what());
+}
     
-Exception::Exception() throw() {}
+Exception::Exception() throw() { ctorLog(this); }
 
-Exception::Exception(const std::string& str) throw() : whatStr(str) {}
+Exception::Exception(const std::string& str) throw()
+    : whatStr(str) { ctorLog(this); }
 
-Exception::Exception(const char* str) throw() : whatStr(str) {}
+Exception::Exception(const char* str) throw() : whatStr(str) { ctorLog(this); }
 
 Exception::~Exception() throw() {}
 
diff --git a/cpp/src/qpid/broker/Daemon.cpp b/cpp/src/qpid/broker/Daemon.cpp
index 3c5a642b60..2b64e7761b 100644
--- a/cpp/src/qpid/broker/Daemon.cpp
+++ b/cpp/src/qpid/broker/Daemon.cpp
@@ -16,50 +16,35 @@
  *
  */
 #include "Daemon.h"
+#include "qpid/log/Statement.h"
 #include "qpid/QpidError.h"
 #include <libdaemon/daemon.h>
 #include <errno.h>
 #include <unistd.h>
 #include <sys/stat.h>
 #include <signal.h>
-#include <boost/filesystem/path.hpp>
-#include <boost/filesystem/operations.hpp>
 
 namespace qpid {
 namespace broker {
 
 using namespace std;
 
-string Daemon::pidFile;
-string Daemon::name;
+boost::function<std::string()> qpid::broker::Daemon::pidFileFn;
 
-string Daemon::nameFromArgv0(const char* argv0) {
-    return string(daemon_ident_from_argv0(const_cast<char*>(argv0)));
+std::string Daemon::defaultPidFile(const std::string& identifier) {
+    daemon_pid_file_ident=identifier.c_str();
+    return daemon_pid_file_proc_default();
 }
 
-const char* Daemon::getPidFile() {
-    if (pidFile.empty()) {
-        const char* home=getenv("HOME");
-        if (!home)
-            throw(Exception("$HOME is not set, cant create $HOME/.qpidd."));
-        using namespace boost::filesystem;
-        path dir = path(home,native) / path(".qpidd", native);
-        create_directory(dir);
-        dir /= name;
-        pidFile = dir.string();
-    }
-    return pidFile.c_str();
+const char* Daemon::realPidFileFn() {
+    static std::string str = pidFileFn();
+    return str.c_str();
 }
 
-Daemon::Daemon(const string& name_, int secs) : pid(-1), timeout(secs)
+Daemon::Daemon(boost::function<std::string()> fn, int secs) : pid(-1), timeout(secs)
 {
-    name = name_;
-    daemon_pid_file_ident = daemon_log_ident = name.c_str();
-    if (getuid() != 0) {
-        // For normal users put pid file under $HOME/.qpid
-        daemon_pid_file_proc = getPidFile;
-    }
-    // For root use the libdaemon default: /var/run.    
+    pidFileFn = fn;
+    daemon_pid_file_proc = &realPidFileFn;
 }
 
 Daemon::~Daemon() {
@@ -77,44 +62,58 @@ class Daemon::Retval {
     bool completed;
 };
 
-pid_t Daemon::fork() {
+pid_t Daemon::fork(Function parent, Function child) {
     retval.reset(new Retval());
     pid = daemon_fork();
     if (pid < 0)
-            throw Exception("Failed to fork daemon: "+strError(errno));
-    else if (pid > 0) {
-        int ret = retval->wait(timeout); // parent, wait for child.
-        if (ret != 0) {
-            string err;
-            if (ret > 0)
-                err = strError(ret);
-            else if (ret == -1)
-                err= strError(errno);
-            else
-                err= "unknown error";
-            throw Exception("Deamon startup failed: "+err);
+        throw Exception("Failed to fork daemon: "+strError(errno));
+    else if (pid == 0) {
+        try {
+            child(*this);
+        } catch (const exception& e) {
+            QPID_LOG(debug, "Rethrowing: " << e.what());
+            failed();           // Notify parent
+            throw;
         }
     }
-    else if (pid == 0) { // child.
-        // TODO aconway 2007-04-26: Should log failures.
-        if (daemon_pid_file_create())
-            failed();
-    }
+    else
+        parent(*this);
     return pid;
 }
 
-void Daemon::notify(int i) {
+int Daemon::wait() {  // parent 
+    assert(retval);
+    errno = 0;                  // Clear errno.
+    int ret = retval->wait(timeout); // wait for child.
+    if (ret == -1) {
+        if (errno)
+            throw Exception("Error waiting for daemon startup:"
+                            +strError(errno));
+        else
+            throw Exception("Error waiting for daemon startup, check logs.");
+    }
+    return ret;
+}
+
+void Daemon::notify(int value) { // child
     assert(retval);
-    if (retval->send(i)) 
+    if (retval->send(value)) 
         throw Exception("Failed to notify parent: "+strError(errno));
 }
 
-void Daemon::ready() { notify(0); }
+void Daemon::ready(int value) { // child
+    if (value==-1)
+        throw Exception("Invalid value in Dameon::notify");
+    errno = 0;
+    if (daemon_pid_file_create() != 0)
+        throw Exception(string("Failed to create PID file ") +
+                        daemon_pid_file_proc()+": "+strError(errno));
+    notify(value);
+}
 
-// NB: Not -1, confused with failure of fork() on the parent side.
-void Daemon::failed() { notify(errno? errno:-2); }
+void Daemon::failed() { notify(-1); }
 
-void  Daemon::quit() {
+void  Daemon::quit() { 
     if (daemon_pid_file_kill_wait(SIGINT, timeout))
         throw Exception("Failed to stop daemon: " + strError(errno));
 }
diff --git a/cpp/src/qpid/broker/Daemon.h b/cpp/src/qpid/broker/Daemon.h
index cf9bf13764..cf7ad37b3a 100644
--- a/cpp/src/qpid/broker/Daemon.h
+++ b/cpp/src/qpid/broker/Daemon.h
@@ -21,6 +21,7 @@
 
 #include <string>
 #include <boost/scoped_ptr.hpp>
+#include <boost/function.hpp>
 
 namespace qpid {
 namespace broker {
@@ -32,24 +33,39 @@ namespace broker {
 class Daemon
 {
   public:
-
-    /** Extract the daemon's name from argv[0] */
-    static std::string nameFromArgv0(const char* argv0);
+    /** Utility function to create pid file name in a standard place
+     * (may require root acces) using identifier as the file name.
+     */
+    static std::string defaultPidFile(const std::string& identifier);
     
     /**
-     * Creating a Daemon instance forks a daemon process.
-     *@param name used to create pid files etc.
-     *@param timeout in seconds for all operations that wait.
+     * Daemon control object.
+     *@param pidFileFn Function that will comupte a PID file name.
+     * Called when pid file is created in ready()
+     *@param timeout in seconds for any operations that wait.
      */
-    Daemon(const std::string& name, int timeout);
+    Daemon(boost::function<std::string()> pidFileFn, int timeout);
 
     ~Daemon();
-    
-    /** Fork the daemon, wait till it signals readiness */
-    pid_t fork();
 
-    /** Child only, send ready signal so parent fork() will return. */
-    void ready();
+    typedef boost::function<void(Daemon&)> Function;
+
+    /** Fork the daemon.
+     *@param parent called in the parent process.
+     *@param child called in the child process.
+     */
+    pid_t fork(Function parent, Function child);
+
+    /** Parent only: wait for child to indicate it is ready.
+     * @return value child passed to ready() */
+    int wait();
+
+    /** Child only. Notify the parent we are ready and write the
+     * PID file.
+     *@param value returned by parent call to wait(). -1 is reserved
+     * for signalling an error.
+     */
+    void ready(int value);
     
     /** Child only, send failed signal so parent fork() will throw. */
     void failed();
@@ -67,18 +83,16 @@ class Daemon
 
     bool isChild() { return pid == 0; }
     
-    std::string getName() const { return name; }
-
     pid_t getPid() const {return pid; }
 
   private:
     class Retval;
 
+    static boost::function<std::string()> pidFileFn;
+    static const char* realPidFileFn();
     void notify(int);
     
-    static std::string name;
-    static std::string pidFile;
-    static const char* getPidFile();
+    static std::string identifier;
     boost::scoped_ptr<Retval> retval;
     pid_t pid;
     int timeout;
@@ -86,6 +100,4 @@ class Daemon
 
 }} // namespace qpid::broker
 
-
-
 #endif  /*!_broker_Daemon_h*/
diff --git a/cpp/src/qpidd.cpp b/cpp/src/qpidd.cpp
index 2f4d16f0a7..3378fcdc96 100644
--- a/cpp/src/qpidd.cpp
+++ b/cpp/src/qpidd.cpp
@@ -25,23 +25,18 @@
 #include "qpid/log/Options.h"
 #include "qpid/log/Logger.h"
 #include "config.h"
+#include <boost/filesystem/path.hpp>
+#include <boost/filesystem/operations.hpp>
 #include <iostream>
 #include <fstream>
 #include <signal.h>
+#include <unistd.h>
 
 using namespace qpid;
 using namespace qpid::broker;
 using namespace qpid::sys;
 using namespace std;
 
-Broker::shared_ptr brokerPtr;
-
-void handle_signal(int /*signal*/){
-    QPID_LOG(notice, "Shutting down...");
-    brokerPtr->shutdown();
-}
-
-
 /** Command line options */
 struct QpiddOptions : public Broker::Options, public log::Options
 {
@@ -52,39 +47,42 @@ struct QpiddOptions : public Broker::Options, public log::Options
     bool quit;
     bool kill;
     bool check;
-    bool ppid;
     int wait;
     string config;
     po::options_description mainOpts;
     po::options_description allOpts;
-    po::options_description logOpts;
     
     QpiddOptions() :
         help(false), version(false), daemon(false),
-        quit(false), kill(false), check(false), ppid(false), wait(10),
+        quit(false), check(false), 
+        wait(10),
         config("/etc/qpidd.conf"),
-        mainOpts("Options"),
-        logOpts("Logging Options")
+        mainOpts("Broker Options")
     {
         using namespace po;
-        mainOpts.add_options()
-            ("daemon,d", optValue(daemon), "Run as a daemon.")
-            ("quit,q", optValue(quit), "Stop the running daemon politely.")
-            ("kill,k", optValue(kill), "Kill the running daemon harshly.")
-            ("check,c", optValue(check), "If daemon is running return 0.")
-            ("wait", optValue(wait, "SECONDS"),
-             "Maximum wait for daemon response.")
-            ("ppid", optValue(ppid), "Print daemon pid to stdout." );
-        po::options_description brokerOpts;
+        // First set up the sub-option groups.
+        options_description daemonOpts("Daemon Options");
+        daemonOpts.add_options()
+            ("daemon,d", optValue(daemon), "Run as a daemon. With --port 0 print actual listening port.")
+            ("wait,w", optValue(wait, "SECONDS"), "Maximum wait for daemon response.")
+            ("check,c", optValue(check), "If a daemon is running print its pid to stdout and return 0.")
+            ("quit,q", optValue(quit), "Stop the running daemon politely.");
+
+        options_description logOpts("Logging Options");
+        log::Options::addTo(logOpts);
+
+        // Populate the main options group for --help
         Broker::Options::addTo(mainOpts);
         mainOpts.add_options()
             ("config", optValue(config, "FILE"), "Configuation file.")
             ("help,h", optValue(help), "Print help message.")
             ("long-help", optValue(longHelp), "Show complete list of options.")
             ("version,v", optValue(version), "Print version information.");
+        mainOpts.add(daemonOpts);
 
-        log::Options::addTo(logOpts);
+        // Populate the all options group
         allOpts.add(mainOpts).add(logOpts);
+
     }
 
     void parse(int argc, char* argv[]) {
@@ -97,19 +95,62 @@ struct QpiddOptions : public Broker::Options, public log::Options
     };
 };
 
+// Globals
+Broker::shared_ptr brokerPtr;
+QpiddOptions config;
+
+void handle_signal(int /*signal*/){
+    QPID_LOG(notice, "Shutting down...");
+    brokerPtr->shutdown();
+}
+
+/** Compute a name for the pid file */
+std::string pidFileFn() {
+    uint16_t port=brokerPtr ? brokerPtr->getPort() : config.port;
+    string file=(boost::format("qpidd.%d.pid") % port).str();
+    string pidPath;
+    if (getuid() == 0)          // Use standard pid file for root.
+        pidPath=Daemon::defaultPidFile(file);
+    else {                      // Use $HOME/.qpidd for non-root.
+        const char* home=getenv("HOME");
+        if (!home)
+            throw(Exception("$HOME is not set, cant create $HOME/.qpidd."));
+        namespace fs=boost::filesystem;
+        fs::path dir = fs::path(home,fs::native) / fs::path(".qpidd", fs::native);
+        fs::create_directory(dir);
+        dir /= file;
+        pidPath=dir.string();
+    }
+    QPID_LOG(debug, "PID file name=" << pidPath);
+    return pidPath;
+}
+
+/** Code for forked parent */
+void parent(Daemon& demon) {
+    uint16_t realPort = demon.wait();
+    if (config.port == 0)
+        cout << realPort << endl; 
+}
+
+/** Code for forked child */
+void child(Daemon& demon) {
+    brokerPtr=Broker::create(config);
+    uint16_t realPort=brokerPtr->getPort();
+    demon.ready(realPort);   // Notify parent.
+    brokerPtr->run();
+}
+
+
 int main(int argc, char* argv[])
 {
-    QpiddOptions config;
+    // Spelled 'demon' to avoid clash with daemon.h function.
+    Daemon demon(pidFileFn, config.wait);
+
     try {
         config.parse(argc, argv);
         if (config.trace)
             config.selectors.push_back("trace+");
         log::Logger::instance().configure(config, argv[0]);
-        string name=(boost::format("%s.%d")
-                     % Daemon::nameFromArgv0(argv[0])
-                     % (config.port)).str();
-        // Spelled 'demon' to avoid clash with daemon.h function.
-        Daemon demon(name, config.wait);
 
         // Options that just print information.
         if(config.help || config.longHelp || config.version) {
@@ -123,46 +164,32 @@ int main(int argc, char* argv[])
             return 0;
         }
 
-        // Options that act on an already running daemon.
-        if (config.quit || config.kill || config.check) {
+        // Stop running daemon
+        if (config.quit) {
+            demon.quit();
+            return 0;
+        }
+
+        // Query running daemon
+        if (config.check) {
             pid_t pid = demon.check();
-            if (config.ppid && pid > 0)
-                cout << pid << endl;
-            if (config.kill)
-                demon.kill();
-            else if (config.quit)
-                demon.quit();
-            if (config.check && pid <= 0)
+            if (pid < 0) 
                 return 1;
-            return 0;
+            else {
+                cout << pid << endl;
+                return 0;
+            }
         }
 
         // Starting the broker:
         signal(SIGINT, handle_signal);
-        if (config.daemon) {
-            pid_t pid = demon.fork();
-            if (pid == 0) {  // Child
-                try {
-                    brokerPtr=Broker::create(config);
-                    demon.ready();   // Notify parent we're ready.
-                    brokerPtr->run();
-                } catch (const exception& e) {
-                    QPID_LOG(critical, "Broker daemon startup failed: " << e.what());
-                    demon.failed(); // Notify parent we failed.
-                    return 1;
-                }
-            }
-            else if (pid > 0) { // Parent
-                if (config.ppid)
-                    cout << pid << endl;
-                return 0;
-            }
-            else { // pid < 0 
-                throw Exception("fork failed"+strError(errno));
-            }
-        } // Non-daemon broker.
-        else {
+        if (config.daemon) {    // Daemon broker
+            demon.fork(parent, child);
+        } 
+        else {                  // Non-daemon broker.
             brokerPtr = Broker::create(config);
+            if (config.port == 0)
+                cout << uint16_t(brokerPtr->getPort()) << endl; 
             brokerPtr->run(); 
         }
         return 0;
@@ -170,12 +197,13 @@ int main(int argc, char* argv[])
     catch(const po::error& e) {
         // Command line parsing error.
         cerr << "Error: " << e.what() << endl
-             << "Type 'qpidd --help' for usage." << endl;
+             << "Type 'qpidd --long-help' for full usage." << endl;
     }
     catch(const exception& e) {
-        // Could be child or parent so log and print.
-        QPID_LOG(error, e.what());
-        cerr << "Error: " << e.what() << endl;
+        if (demon.isParent())
+            cerr << "Error: " << e.what() << endl;
+        else
+            QPID_LOG(critical, e.what());
     }
     return 1;
 }
diff --git a/cpp/src/tests/daemon_test b/cpp/src/tests/daemon_test
index ab87a2f848..6d1fc73923 100755
--- a/cpp/src/tests/daemon_test
+++ b/cpp/src/tests/daemon_test
@@ -4,21 +4,28 @@
 #
 
 TEMP=`mktemp`
-qpidd=../qpidd
+qpidd="../qpidd --log.output qpidd.log"
 client_tests=./client_test
 trap 'rm -f $TEMP' 0
 
 fail() { echo FAIL: $0:$* 1>&2; exit 1; }
 
-# Start and stop daemon.
-PID=`$qpidd --check --ppid` && fail $LINENO: $qpidd already running $PID
-$qpidd -d || $LINENO: $qpidd -d failed
-$qpidd --check || fail $LINENO: $qpidd --check says $qpidd didnt start
+# Start and stop daemon on default port.
+PID=`$qpidd --check` && fail $LINENO: qpidd already running pid=$PID
+$qpidd -d || fail $LINENO: $qpidd -d failed
+$qpidd -c >/dev/null || fail $LINENO: qpidd --check says qpidd did not start
 ./client_test > $TEMP || fail $LINENO:  client_test: `cat $TEMP`
-$qpidd -q || fail $LINENO: $qpidd -q failed
-$qpidd -d || fail $LINENO: restart after quit failed.
-$qpidd -k || fail $LINENO: $qpidd -k failed
-# Supress expected message re. cleanup of old PID file.
-PID=`$qpidd --check --ppid 2>/dev/null` && fail $LINENO: $PID still running after kill. 
+$qpidd -q || fail $LINENO: qpidd -q failed
+$qpidd -c >/dev/null && fail $LINENO: Still running after quit.
+
+# Start and stop daemon on dynamic port.
+export QPID_PORT=`$qpidd -dp0`
+# Note: QPID_PORT fom environment will be used below here:
+$qpidd -c >/dev/null || fail $LINENO: qpidd did not start. QPID_PORT=$QPID_PORT
+$qpidd -q || fail $LINENO: qpidd -q failed. QPID_PORT=$QPID_PORT
+$qpidd -c >/dev/null && fail $LINENO: Still running after start. QPID_PORT=$QPID_PORT
+
+# FIXME aconway 2007-06-11: run client test, needs a --port option.
+
 
 true