changes to AssignmentClient architecture to fix fork behaviour

assignment-client/src/AssignmentClient.cpp

@@ -24,12 +24,8 @@ const long long ASSIGNMENT_REQUEST_INTERVAL_MSECS = 1 * 1000;
 
 int hifiSockAddrMeta = qRegisterMetaType<HifiSockAddr>("HifiSockAddr");
 
-AssignmentClient::AssignmentClient(int &argc, char **argv,
+AssignmentClient::AssignmentClient(int &argc, char **argv) :
-                                   Assignment::Type requestAssignmentType,
-                                   const HifiSockAddr& customAssignmentServerSocket,
-                                   const char* requestAssignmentPool) :
     QCoreApplication(argc, argv),
-    _requestAssignment(Assignment::RequestCommand, requestAssignmentType, requestAssignmentPool),
     _currentAssignment(NULL)
 {
     // register meta type is required for queued invoke method on Assignment subclasses
@@ -37,12 +33,53 @@ AssignmentClient::AssignmentClient(int &argc, char **argv,
     // set the logging target to the the CHILD_TARGET_NAME
     Logging::setTargetName(ASSIGNMENT_CLIENT_TARGET_NAME);
     
+    const char ASSIGNMENT_TYPE_OVVERIDE_OPTION[] = "-t";
+    const char* assignmentTypeString = getCmdOption(argc, (const char**)argv, ASSIGNMENT_TYPE_OVVERIDE_OPTION);
+    
+    Assignment::Type requestAssignmentType = Assignment::AllTypes;
+    
+    if (assignmentTypeString) {
+        // the user is asking to only be assigned to a particular type of assignment
+        // so set that as the ::overridenAssignmentType to be used in requests
+        requestAssignmentType = (Assignment::Type) atoi(assignmentTypeString);
+    }
+    
+    const char ASSIGNMENT_POOL_OPTION[] = "--pool";
+    const char* requestAssignmentPool = getCmdOption(argc, (const char**) argv, ASSIGNMENT_POOL_OPTION);
+    
+    
+    // setup our _requestAssignment member variable from the passed arguments
+    _requestAssignment = Assignment(Assignment::RequestCommand, requestAssignmentType, requestAssignmentPool);
+    
     // create a NodeList as an unassigned client
     NodeList* nodeList = NodeList::createInstance(NODE_TYPE_UNASSIGNED);
     
+    const char CUSTOM_ASSIGNMENT_SERVER_HOSTNAME_OPTION[] = "-a";
+    const char CUSTOM_ASSIGNMENT_SERVER_PORT_OPTION[] = "-p";
+    
+    // grab the overriden assignment-server hostname from argv, if it exists
+    const char* customAssignmentServerHostname = getCmdOption(argc, (const char**)argv, CUSTOM_ASSIGNMENT_SERVER_HOSTNAME_OPTION);
+    const char* customAssignmentServerPortString = getCmdOption(argc,(const char**)argv, CUSTOM_ASSIGNMENT_SERVER_PORT_OPTION);
+    
+    HifiSockAddr customAssignmentSocket;
+    
+    if (customAssignmentServerHostname || customAssignmentServerPortString) {
+        
+        // set the custom port or default if it wasn't passed
+        unsigned short assignmentServerPort = customAssignmentServerPortString
+        ? atoi(customAssignmentServerPortString) : DEFAULT_DOMAIN_SERVER_PORT;
+        
+        // set the custom hostname or default if it wasn't passed
+        if (!customAssignmentServerHostname) {
+            customAssignmentServerHostname = DEFAULT_ASSIGNMENT_SERVER_HOSTNAME;
+        }
+        
+        customAssignmentSocket = HifiSockAddr(customAssignmentServerHostname, assignmentServerPort);
+    }
+    
     // set the custom assignment socket if we have it
-    if (!customAssignmentServerSocket.isNull()) {
+    if (!customAssignmentSocket.isNull()) {
-        nodeList->setAssignmentServerSocket(customAssignmentServerSocket);
+        nodeList->setAssignmentServerSocket(customAssignmentSocket);
     }
     
     // call a timer function every ASSIGNMENT_REQUEST_INTERVAL_MSECS to ask for assignment, if required

assignment-client/src/AssignmentClient.h

@@ -16,10 +16,7 @@
 class AssignmentClient : public QCoreApplication {
     Q_OBJECT
 public:
-    AssignmentClient(int &argc, char **argv,
+    AssignmentClient(int &argc, char **argv);
-                     Assignment::Type requestAssignmentType = Assignment::AllTypes,
-                     const HifiSockAddr& customAssignmentServerSocket = HifiSockAddr(),
-                     const char* requestAssignmentPool = NULL);
 private slots:
     void sendAssignmentRequest();
     void readPendingDatagrams();

assignment-client/src/AssignmentClientMonitor.cpp

@@ -0,0 +1,52 @@
+//
+//  AssignmentClientMonitor.cpp
+//  hifi
+//
+//  Created by Stephen Birarda on 1/10/2014.
+//  Copyright (c) 2014 HighFidelity, Inc. All rights reserved.
+//
+
+#include <Logging.h>
+
+#include "AssignmentClientMonitor.h"
+
+const char* NUM_FORKS_PARAMETER = "-n";
+
+const char ASSIGNMENT_CLIENT_MONITOR_TARGET_NAME[] = "assignment-client-monitor";
+
+AssignmentClientMonitor::AssignmentClientMonitor(int &argc, char **argv, int numAssignmentClientForks) :
+    QCoreApplication(argc, argv)
+{
+    // start the Logging class with the parent's target name
+    Logging::setTargetName(ASSIGNMENT_CLIENT_MONITOR_TARGET_NAME);
+    
+    _childArguments = arguments();
+    
+    // remove the parameter for the number of forks so it isn't passed to the child forked processes
+    int forksParameterIndex = _childArguments.indexOf(NUM_FORKS_PARAMETER);
+    
+    // this removes both the "-n" parameter and the number of forks passed
+    _childArguments.removeAt(forksParameterIndex);
+    _childArguments.removeAt(forksParameterIndex);
+    
+    // use QProcess to fork off a process for each of the child assignment clients
+    for (int i = 0; i < numAssignmentClientForks; i++) {
+        spawnChildClient();
+    }
+}
+
+void AssignmentClientMonitor::spawnChildClient() {
+    QProcess *assignmentClient = new QProcess(this);
+    assignmentClient->start(applicationFilePath(), _childArguments);
+    
+    // link the child processes' finished slot to our childProcessFinished slot
+    connect(assignmentClient, SIGNAL(finished(int, QProcess::ExitStatus)), this,
+            SLOT(childProcessFinished(int, QProcess::ExitStatus)));
+    
+    qDebug() << "Spawned a child client with PID" << assignmentClient->pid() << "\n";
+}
+
+void AssignmentClientMonitor::childProcessFinished(int exitCode, QProcess::ExitStatus exitStatus) {
+    qDebug() << "Replacing dead child assignment client with a new one.\n";
+    spawnChildClient();
+}

assignment-client/src/AssignmentClientMonitor.h

@@ -0,0 +1,31 @@
+//
+//  AssignmentClientMonitor.h
+//  hifi
+//
+//  Created by Stephen Birarda on 1/10/2014.
+//  Copyright (c) 2014 HighFidelity, Inc. All rights reserved.
+//
+
+#ifndef __hifi__AssignmentClientMonitor__
+#define __hifi__AssignmentClientMonitor__
+
+#include <QtCore/QCoreApplication>
+#include <QtCore/QProcess>
+
+#include <Assignment.h>
+
+extern const char* NUM_FORKS_PARAMETER;
+
+class AssignmentClientMonitor : public QCoreApplication {
+    Q_OBJECT
+public:
+    AssignmentClientMonitor(int &argc, char **argv, int numAssignmentClientForks);
+private slots:
+    void childProcessFinished(int exitCode, QProcess::ExitStatus exitStatus);
+private:
+    void spawnChildClient();
+    
+    QStringList _childArguments;
+};
+
+#endif /* defined(__hifi__AssignmentClientMonitor__) */

assignment-client/src/main.cpp

@@ -6,172 +6,33 @@
 //  Copyright (c) 2013 High Fidelity, Inc. All rights reserved.
 //
 
-#include <arpa/inet.h>
-#include <errno.h>
-#include <signal.h>
-#include <sys/time.h>
-#include <sys/wait.h>
-
 #include <Logging.h>
-#include <NodeList.h>
-#include <PacketHeaders.h>
 #include <SharedUtil.h>
-#include <VoxelServer.h>
 
-#include "Agent.h"
 #include "Assignment.h"
 #include "AssignmentClient.h"
-#include "audio/AudioMixer.h"
+#include "AssignmentClientMonitor.h"
-#include "avatars/AvatarMixer.h"
-
-const char PARENT_TARGET_NAME[] = "assignment-client-monitor";
-
-pid_t* childForks = NULL;
-HifiSockAddr customAssignmentSocket;
-int numForks = 0;
-Assignment::Type overiddenAssignmentType = Assignment::AllTypes;
-const char* assignmentPool = NULL;
-
-int argc = 0;
-char** argv = NULL;
-
-int childClient() {
-    AssignmentClient client(::argc, ::argv, ::overiddenAssignmentType, customAssignmentSocket, ::assignmentPool);
-    return client.exec();
-}
-
-void sigchldHandler(int sig) {
-    pid_t processID;
-    int status;
-    
-    while ((processID = waitpid(-1, &status, WNOHANG)) != -1) {
-        if (processID == 0) {
-            // there are no more children to process, break out of here
-            break;
-        }
-        
-        int newForkProcessID = 0;
-        
-        // find the dead process in the array of child forks
-        for (int i = 0; i < ::numForks; i++) {
-            if (::childForks[i] == processID) {
-                
-                newForkProcessID = fork();
-                if (newForkProcessID == 0) {
-                    // this is the child, call childClient
-                    childClient();
-                    
-                    // break out so we don't fork bomb
-                    break;
-                } else {
-                    // this is the parent, replace the dead process with the new one
-                    ::childForks[i] = newForkProcessID;
-                   
-                    qDebug("Replaced dead %d with new fork %d\n", processID, newForkProcessID);
-                    
-                    break;
-                }
-            }
-        }
-    }
-    
-}
-
-void parentMonitor() {
-    
-    struct sigaction sa;
-    
-    memset(&sa, 0, sizeof(sa));
-    sa.sa_handler = sigchldHandler;
-    
-    sigaction(SIGCHLD, &sa, NULL);
-    
-    pid_t childID = 0;
-    
-    // don't bail until all children have finished
-    while ((childID = waitpid(-1, NULL, 0))) {
-        if (errno == ECHILD) {
-            break;
-        }
-    }
-    
-    // delete the array of pid_t holding the forked process IDs
-    delete[] ::childForks;
-}
 
 int main(int argc, char* argv[]) {
-    ::argc = argc;
-    ::argv = argv;
     
     setvbuf(stdout, NULL, _IOLBF, 0);
     
     // use the verbose message handler in Logging
     qInstallMessageHandler(Logging::verboseMessageHandler);
     
-    // start the Logging class with the parent's target name
-    Logging::setTargetName(PARENT_TARGET_NAME);
-    
-    const char CUSTOM_ASSIGNMENT_SERVER_HOSTNAME_OPTION[] = "-a";
-    const char CUSTOM_ASSIGNMENT_SERVER_PORT_OPTION[] = "-p";
-    
-    // grab the overriden assignment-server hostname from argv, if it exists
-    const char* customAssignmentServerHostname = getCmdOption(argc, (const char**)argv, CUSTOM_ASSIGNMENT_SERVER_HOSTNAME_OPTION);
-    const char* customAssignmentServerPortString = getCmdOption(argc,(const char**)argv, CUSTOM_ASSIGNMENT_SERVER_PORT_OPTION);
-    
-    if (customAssignmentServerHostname || customAssignmentServerPortString) {
-        
-        // set the custom port or default if it wasn't passed
-        unsigned short assignmentServerPort = customAssignmentServerPortString
-            ? atoi(customAssignmentServerPortString) : DEFAULT_DOMAIN_SERVER_PORT;
-        
-        // set the custom hostname or default if it wasn't passed
-        if (!customAssignmentServerHostname) {
-            customAssignmentServerHostname = DEFAULT_ASSIGNMENT_SERVER_HOSTNAME;
-        }
-        
-        ::customAssignmentSocket = HifiSockAddr(customAssignmentServerHostname, assignmentServerPort);
-    }
-    
-    const char ASSIGNMENT_TYPE_OVVERIDE_OPTION[] = "-t";
-    const char* assignmentTypeString = getCmdOption(argc, (const char**)argv, ASSIGNMENT_TYPE_OVVERIDE_OPTION);
-    
-    if (assignmentTypeString) {
-        // the user is asking to only be assigned to a particular type of assignment
-        // so set that as the ::overridenAssignmentType to be used in requests
-        ::overiddenAssignmentType = (Assignment::Type) atoi(assignmentTypeString);
-    }
-    
-    const char ASSIGNMENT_POOL_OPTION[] = "--pool";
-    ::assignmentPool = getCmdOption(argc, (const char**) argv, ASSIGNMENT_POOL_OPTION);
-
-    const char* NUM_FORKS_PARAMETER = "-n";
     const char* numForksString = getCmdOption(argc, (const char**)argv, NUM_FORKS_PARAMETER);
     
-    int processID = 0;
+    int numForks = 0;
     
     if (numForksString) {
-        ::numForks = atoi(numForksString);
+        numForks = atoi(numForksString);
-        qDebug("Starting %d assignment clients\n", ::numForks);
-        
-        ::childForks = new pid_t[::numForks];
-        
-        // fire off as many children as we need (this is one less than the parent since the parent will run as well)
-        for (int i = 0; i < ::numForks; i++) {
-            processID = fork();
-            
-            if (processID == 0) {
-                // this is in one of the children, break so we don't start a fork bomb
-                break;
-            } else {
-                // this is in the parent, save the ID of the forked process
-                childForks[i] = processID;
-            }
-        }
     }
     
-    if (processID == 0 || ::numForks == 0) {
+    if (numForks) {
-        return childClient();
+        AssignmentClientMonitor monitor(argc, argv, numForks);
+        return monitor.exec();
     } else {
-        parentMonitor();
+        AssignmentClient client(argc, argv);
+        return client.exec();
     }
 }