0

I wondered how should i approach the following problem. My c program will receive 3 arguments - path, term and number of threads to invoke.

I should explore files that contain the term in their name, in any of the sub-directories of the path. If my program would have been asychronous it would have been a rather easy job.

I am using a shared queue for my threads, with a lock and condition variable for synchronization.

My logic goes as follows - 

int main(int argc, char *argv)
{
 ...
Queue *queue = init_queue();
enqueue(queue, root_path);    

for (int i = 0; i<n_threads; ++i)
    pthread_create(..., thread_handle, ...);
 ...
}

void thread_handle()
{

    while (!queue_empty)
    {
          while(!condition_variable)
              pthread_cond_wait(&cond);
          pthread_mutex_lock(&lock);
          QNode *node = dequeue(queue);  
          iterate_dir(node->path);
          pthread_mutex_unlock(&lock);
          thread_cond_signal(condition_variable);
    }
}

void iterate_dir(...)
{
    // Directory search logic (with enqueue..)
}

This is rather a psuedo-code than a real code, but I'm more worried about my logic than my implementation. My question is, how can i signal my threads that empty queue signals to end their function, and it's not just temporary until the queue will contain some path.

I would love to hear you opinions!

Yariv Levy
  • 162
  • 1
  • 9
  • 2
    Your entrance is wrong. The mutex should be locked before invoking `pthread_cond_wait` (and in fact, before entrance to the loop itself), and further, it is already locked upon return of that call, barring errors. At the risk of sounding self-serving, [read this](https://stackoverflow.com/questions/14924469/does-pthread-cond-waitcond-t-mutex-unlock-and-then-lock-the-mutex/14925150#14925150). – WhozCraig Jan 05 '20 at 14:57
  • The code not shown does initialise `cond` and `mute`, doesn't it? – alk Jan 05 '20 at 14:58
  • 1
    You shouldn’t recurse in `iterate_dir()` with the mutex locked, should you? It is going to limit the concurrency. – Jonathan Leffler Jan 05 '20 at 16:04

1 Answers1

1

I'm more worried about my logic than my implementation. My question is, how can i signal my threads that empty queue signals to end their function, and it's not just temporary until the queue will contain some path.

I think you're asking how to deal with the fact that the queue being empty does not constitute an appropriate thread-termination condition, because new directories might be enqueued by any threads that are still active. The answer is simple: don't use queue emptiness as the (sole) termination condition. Instead, maintain another shared variable that tracks the number of threads currently processing directories -- also under protection of the mutex -- and make the loop termination condition be that the queue is empty and no threads are processing directories.

Note also that you must

  • take care to ensure that all accesses to shared state are protected by the mutex. Including the queue-emptiness test in the loop condition.
  • take care to limit the scope of the mutex as much as possible, so as to allow the greatest amount of concurrency.
  • be prepared to handle spurious wakeups.

The logic might look more like this:

// ...

int num_active_threads = 0;

void *thread_handle(void *arg) {
    pthread_mutex_lock(&lock);
    while (true) {
        while (queue_empty && num_active_threads > 0) {
                pthread_cond_wait(&cond);
        }
        if (queue_empty) break; // it must (also) be that num_active_threads == 0, so all done
        QNode *node = dequeue(queue);  
        num_active_threads++;
        pthread_mutex_unlock(&lock);

        iterate_dir(node->path);

        pthread_mutex_lock(&lock);
        num_active_threads--;
        pthread_cond_broadcast(condition_variable);
    }
    pthread_mutex_unlock(&lock);
}

void iterate_dir(...)
{
    // Directory search logic (with MUTEX-PROTECTED enqueue..)
}
John Bollinger
  • 160,171
  • 8
  • 81
  • 157