3

I'm writing a small utility which is supposed to launch several commands in parallel using system() and wait for their results for logging purposes. However, even though I'm calling system() on different threads, by looking at my Activity Monitor I only see one instance of each command at a time. It looks like system is internally synchronized on a mutex, and only one execution is allowed at each time, however this looks like a huge limitation, can someone confirm this behavior? Do you have any ideas on how to solve it?

Update by looking at the threads execution flow, it looks like they're effectively synchronizing on a mutex. Is there an alternative system() which doesn't do that?

threads

I should mention I'm using C++11 (w/ clang and libc++) on Mac OS 10.7.5.

Update code is:

void Batch::run()
{
    done.clear();
    generator->resetGeneration();

    while(generator->hasMoreParameters())
    {
        // Lock for accessing active
        unique_lock<mutex> lock(q_mutex, adopt_lock);

        // If we've less experiments than threads
        if (active.size() < threads)
        {
            Configuration conf = generator->generateParameters();
            Experiment e(executable, conf);

            thread t(&Experiment::run, e, reference_wrapper<Batch>(*this));
            thread::id id = t.get_id();
            active.insert(id);
            t.detach();
        }

        // Condition variable
        q_control.wait(lock, [this] { return active.size() < threads; } );

    }
}

void Batch::experimentFinished(std::thread::id pos)
{
    unique_lock<mutex> lock(q_mutex, adopt_lock);
    active.erase(pos);
    lock.unlock();
    q_control.notify_all();
}

void Experiment::run(Batch& caller)
{    
    // Generate run command
    stringstream run_command;
    run_command << executable + " ";
    ParameterExpression::printCommandLine(run_command, config);

    if (system(run_command.str().c_str()))
        stats["success"] = "true";
    else
        stats["success"] = "false";

    caller.experimentFinished(this_thread::get_id());
}

Just to be clear: the spawning and handling of threads works fine and does what it needs to do, but it looks like you can have just one instance of system() running at a time.

Thanks

ildjarn
  • 59,718
  • 8
  • 115
  • 201
tunnuz
  • 21,380
  • 29
  • 86
  • 124

3 Answers3

3

POSIX has this to say about system(3):

Using the system() function in more than one thread in a process or when the SIGCHLD signal is being manipulated by more than one thread in a process may produce unexpected results.

Due to the way that SIGCHLD must be blocked during the execution, running system calls concurrently doesn't really work. If you want multiple threads to run external tasks, you'll need to write a bit more code (handling fork/exec/wait yourself).

Mat
  • 188,820
  • 38
  • 367
  • 383
  • Can you suggest a solution in particular? – tunnuz Oct 11 '12 at 19:05
  • That's non-trivial, there's quite a bit of book-keeping you need to do, and doing it thread-safely could be tricky (see http://www.linuxprogrammingblog.com/threads-and-fork-think-twice-before-using-them for interesting issues). I'm not sure I'd know how to write that correctly, so I'd use a library and hope it got it right (see http://stackoverflow.com/questions/1683665/where-is-boost-process). – Mat Oct 11 '12 at 19:18
  • I posted the solution that I used in an old program that I created below. It doesn't use multiple threads in the same process, it creates a new process via `fork`. – Geoff Montee Oct 11 '12 at 19:24
  • @Geoff_Montee: you should take a look at the first link in my comment. The problem is doing the fork/exec/wait dance from multiple threads which happens to get tricky pretty fast. – Mat Oct 11 '12 at 19:26
  • Yeah, that's why I included the disclaimer. I'm not recommending that it be used as-is in a multi-threaded program. It's more of an option in case multi-process can be used in place of multi-threaded. But I don't know much about the OP's use case. – Geoff Montee Oct 11 '12 at 19:29
2

To whoever comes later, popen did the trick, as it doesn't internally keep a mutex. The code to make it work is

FILE* proc;
char buff[1024];

// Keep track of the success or insuccess of execution
if (!(proc = popen(run_command.str().c_str(), "r")))
    stats["success"] = "false";
else
    stats["success"] = "true";

// Exhaust output
while(fgets(buff, sizeof(buff), proc) != nullptr);

pclose(proc);
tunnuz
  • 21,380
  • 29
  • 86
  • 124
  • Nice. Still need to be sure you have nothing else that calls waitpid/waitid with a process selection parameter that includes the `popen`'d process, but that sounds like a great workaround. – Mat Oct 12 '12 at 08:42
  • Sure. However, I think I'm going to self-accept the answer for this time, since it is a possible solution of the explained problem. – tunnuz Oct 12 '12 at 12:48
1

In case this helps, I wrote some fork/exec/wait code in C++ a while back. It captures output into a std::string.

As @Mat points out, fork, exec, and wait aren't really designed to be uses in a multi-threaded process.

So this is more useful if multi-process can be a substitute for multi-threaded in your application.

bool Utility::execAndRedirect(std::string command, std::vector<std::string> args,     std::string& output, int& status)
{
    int error;
    int pipefd[2];
    int localStatus;

    if (pipe(pipefd) == -1)
    {
        error = errno;
        cerr << "Executing command '" << command << "' failed: " << strerror(error) << endl;
        return false;
    }

    pid_t pid = fork();

    if (pid == 0)
    {       
        char** argsC;

        argsC = new char*[args.size() + 2];

        argsC[0] = new char[command.size() + 1];

        strncpy(argsC[0], command.c_str(), command.size());

        argsC[0][command.size()] = '\0';

        for (size_t count = 0; count < args.size(); count++)
        {
            argsC[count + 1] = new char[args[count].size() + 1];

            strncpy(argsC[count + 1], args[count].c_str(), args[count].size());

            argsC[count + 1][args[count].size()] = '\0';            
        }

        argsC[args.size() + 1] = NULL;

        close(pipefd[0]); 

        if (dup2(pipefd[1], STDOUT_FILENO) == -1)
        {
            error = errno;
            cerr << "Executing command '" << command << "' failed: " << strerror(error) << endl;
            exit(1);        
        }       

        if (dup2(pipefd[1], STDERR_FILENO) == -1)
         {
            error = errno;
            cerr << "Executing command '" << command << "' failed: " << strerror(error) << endl;
            exit(1);        
        }       

        close(pipefd[1]);

        if (execvp(command.c_str(), argsC) == -1)
        {
            error = errno;
            cerr << "Executing command '" << command << "' failed: " << strerror(error) << endl;
            exit(1);
        }
    }

    else if (pid > 0)
    {
        size_t BUFFER_SIZE = 1024;
        char buffer[BUFFER_SIZE + 1];

        close(pipefd[1]);

        ostringstream oss;

        ssize_t num_b;

        while ((num_b = read(pipefd[0], buffer, BUFFER_SIZE)) != 0)
        {
            buffer[num_b] = '\0';

            oss << buffer;
        }

        output = oss.str();

        waitpid(pid, &localStatus, 0);

        close(pipefd[0]);
    }

    else
    {
        error = errno;
        cerr << "Executing command '" << command << "' failed: " << strerror(error) << endl;
        return false;   
    }

    if(WIFEXITED(localStatus))
    {
        status = WEXITSTATUS(localStatus);

        //DateTime current = DateTime::now(); //this is a custom class

        if(status == 0)
        {
            return true;
        }

        else
        {
             return false;
        }
    }

    else
    {
        error = errno;
        cerr << "Executing command '" << command << "' failed: child didn't terminate normally" << endl;
        return false;   
    }   
}
Geoff Montee
  • 2,537
  • 11
  • 14