0

I have the following complete, compilable example (also available as a gist) which spawns a small number of threads (echo pass) and one other (echo block; sleep 1; echo unblock). The second is so names because when I see block in the output, the loop just spins its wheels even though the other 'pass' threads should have finished long before then. I would have expected it to output pass however many times, with interspersed checking (since the main thread is looping at this point) and then unblock at the very end.

How can I get the expected behavior?

I realize this may very well be a symptom of using system, but I've been using that to stand up a multithreading solution since I haven't received a working alternative yet. I note a couple of things that I should probably change, but I'm still researching those solutions. They may also be the source of the bug, though.

// -*- compile-command: "g++ -std=c++0x main.cpp;./a.out" -*-
#include <iostream>
#include <thread>
#include <map>
#include <unistd.h>

typedef char status_t;

/**
 * I know I'm not supposed to be using system, but I don't know what
 * else to use in this context.  The program I'm starting is,
 * theoretically, arbitrary.
 *
 * @param command a shell command to run, such as "pdflatex file.tex"
 * @param running a sentinal boolean for whether this thread is still
 *                running
 * @param exit_code_buf a buffer for the exit code
 *
 * @todo use std::{atomic,future,promise} appropriately...
 */
void run(std::string command, bool *running, status_t *exit_code_buf)
{
  // TODO: add std::chrono::duration timeout
  *running = true;
  *exit_code_buf = system(command.c_str());
  *running = false;
}


class ProcessGroup
{
  std::map<std::pair<std::thread *, bool *>, status_t *> threads;
public:
  void Add(std::string);
  void Join();
};

/**
 * Starts a process in a new thread and adds it to the map
 *
 * @param command a command to start
 *
 * @todo use std::{atomic,future,promise} appropriately...
 */
void
ProcessGroup::Add(std::string command)
{
  status_t *status = new status_t(0);
  bool *running = new bool(false);
  std::thread *t = new std::thread(&run, command, running, status);
  threads.insert(std::make_pair(std::make_pair(t, running), status));
}

/**
 * Periodically checks all threads to see if they are still running.
 * If one of them is, sleep for a small amount of time and check
 * again.
 */
void
ProcessGroup::Join()
{
  bool still_running;
  do {
    still_running = false;
    for (auto it = threads.cbegin(); it != threads.cend(); ++it) {
      still_running |= *it->first.second;
    }
    std::cout << "checking" << std::endl;
    if (still_running) usleep (100000);
  } while (still_running);
}

int main()
{
  std::cout << "Program start." << std::endl;
  ProcessGroup pg;
  std::string
    block("echo block; sleep 1; echo unblock"),
    pass("echo pass");

  pg.Add(block);
  std::cout << "here" << std::endl;
  for (int i = 0; i<10; i++) {
    pg.Add(pass);
  }

  std::cout << "Joining threads..." << std::endl;
  pg.Join();
  std::cout << "Program end." << std::endl;
  return 0;
}

compiling…

$ g++ -std=c++0x main.cpp; ./a.out 
Program start.
here
Joining threads...
checking
block
checking
checking
checking
checking
checking
checking
checking
checking
checking
unblock
pass
pass
checking
pass
pass
pass
pass
pass
pass
pass
pass
checking
Program end.

Compilation finished at Fri Oct  3 23:49:45
Sean Allred
  • 3,558
  • 3
  • 32
  • 71
  • This is slightly related to http://stackoverflow.com/q/26179175/1443496, but I imagine it's not the same issue in the slightest. – Sean Allred Oct 04 '14 at 03:49
  • This looks very much like the symptom of using `system()`. Very clearly, while a `system()` call is sitting in `sleep`, all the other `system()` calls are blocked. Besides that, your program is a veritable racetrack - there are data races galore, accessing shared memory with nary a hint of synchronization. This is not the immediate cause of your problem though. – Igor Tandetnik Oct 04 '14 at 05:03
  • I have this hunch that it appears that "pass" is being done after the block is finished (on the console) but they are actually being done while the sleep is occurring. They may appear after because the lines were buffered and were dumped later on. – Michael Petch Oct 04 '14 at 05:49
  • 1
    @MichaelPetch: `endl` calls `flush`. – Igor Tandetnik Oct 04 '14 at 12:46
  • @IgorTandetnik I wasn't intending for the above to be a best-practice example `;)` But I am very much still learning about practical multithreading (most of my 'experience' is in CS research). I'd always thought *accessing* memory (just reading it) was never a problem; the issue would arise in *writing* to it. Should I perhaps add a scoped lock on the boolean to the check in the `for` loop (and of course matching ones in `run`)? – Sean Allred Oct 04 '14 at 12:50
  • 1
    You are in fact writing to `*running` on one thread while reading from the same location on another - that's a race. You could use an `std::atomic_flag` there. Also, initialize `*running` to `true` before kicking off the thread. As written, it is possible for the loop in `Join` to run first and conclude that all threads are finished, before any thread had the chance to set `*running`. – Igor Tandetnik Oct 04 '14 at 13:03
  • I figured he knew about the atomicity issue since there is this comment in the code `* @todo use std::{atomic,future,promise} appropriately...` – Michael Petch Oct 04 '14 at 13:55
  • What I can say is the code works (within the context of not having any locking on global resources) on my Debian box. Although I had to add "-pthread" at compile time to make it function properly. – Michael Petch Oct 04 '14 at 13:57

0 Answers0