ansaurus

Question

A very simple thread pool using pthreads in C++

Answer 1

+2 A:

I'm missing an extern "C" {} block for your thread function at least, since a C library would expect a C ABI. Other than that, I can't see anything obvious.

E.g. create a prototype like:

extern "C" {
    void *workerThread(void *threadarg);
}

Maister 2010-08-24 21:25:49

Answer 2

+3 A:

You are using a busy loop:

  if (inQueue.size()==0) { somethingTodo=false; }
  else
    {
     workOnMe = inQueue.front();
     inQueue.pop_front();
    }
  pthread_mutex_unlock( &inQueueLock );

  if (!somethingTodo) continue;

look up condition variable.
That way your thread is not consuming resources waiting for work to appear in the queue.
See this question

You tagged the question C++ but you are using C style casts.
Also note that in C++ you do not need to add struct here.

my_data = (struct thread_detail *) threadarg;

Technically you should declare your callback function to use the C ABI (as this is a C library.

extern "C" void *workerThread(void *threadarg);

Personal choice move the * beside the type (but that is just my personal pref).

You are not using RAII. So your lock/unlock senarios are not exception safe.

  pthread_mutex_lock( &inQueueLock );

  // Stuff that could throw.

  pthread_mutex_unlock( &inQueueLock );

Even if stuff in the middle can not throw now. You are assuming that somebody will not add code that that does not throw in the future. Make it safe by creating a lock object.

Martin York 2010-08-24 21:33:49

The user named "Dima" tagged the question as C++, not me. Regarding the "busy loop" comment, the worker thread is only called if the queue is non-empty, and once its empty the worker thread terminates. So it's never wasting resources on an empty queue. I think maybe you're thinking my code is more sophisticated than it is. The worker thread pool only starts when there's a job to do, and then it gets killed. This does not seem to address the helgrind output. Judging from people's feedback so far is sounds like helgrind does not give reliable information, even on "good" pthread code?

Ryan Budney 2010-08-24 22:27:57

Regarding your latter comment on the mutex lock/unlock calls -- for now I just want to ensure this stand-alone experiment runs properly. This is not code that somebody else is going to touch. I'm an amateur programmer. I'm okay not complying with RAII. This is just about me understanding pthreads for now.

Ryan Budney 2010-08-24 22:36:13

@Ryan: It would be clearer to say `if ( ! somethingToDo ) break;` rather than `continue`, since the `continue` leads immediately to a loop exit.

Potatoswatter 2010-08-24 22:41:14

@ Ryan Budney: If you writting primerly in C then I withdraw all the comments and consider it good for a C program (though slightly untidy). But I would highlite the point the Potatoswatter made about the use of continue.

Martin York 2010-08-25 00:30:35

I replaced continue with break. I'm still confused as to why helgrind doesn't like this program. Thanks for all the comments.

Ryan Budney 2010-08-25 00:57:25

Answer 3

+1 A:

Several of your errors come from libgcc_s.so. Some of them appear to occur during thread initialization, before your function is even called.

Try compiling with gcc -pthread to make sure the compiler knows what's going on.

Potatoswatter 2010-08-24 22:56:23

That's what I'm doing. Don't you get the same problems if you run helgrind on the executable?

Ryan Budney 2010-08-24 23:04:42

I've notice the application "pbzip2" uses a thread pool, it's also written in a C/C++ style like my code. Helgrind has only one complaint when pbzip2 is running, it's same as the topmost complaint in my helgrind output. So that complaint is perhaps not so important, but the others I don't know what they mean.

Ryan Budney 2010-08-25 02:48:50

@Ryan: The other errors mention `pthread_once`, which is used to initialize static variables. Also, we know they occur before the loop in the workers. Since the only interesting thing there is creation of a `stringstream` and a few `string`s, try building a dummy `stringstream` and doing something similar in `main` before `pthread_create`, just to initialize the related globals. Also, it might be worthwhile to try different versions of the compiler.

Potatoswatter 2010-08-25 04:34:50

I'm using gcc 4.3.3 (under Ubuntu 10.04) at home. I also have gcc 4.2.4 installed at home. Both have the same problem. On the office computer I have 4.4.3 and 4.3.4 installed, both with the same problem. Also running Ubuntu 10.04 at the office. I eliminated the stringstream initializations and calls before thread initialization, with no change in the valgrind output. I'll try similar tweaks to the other things that appear before initialization of the threads.

Ryan Budney 2010-08-25 05:36:14

I'll see if I can try it on some non-Ubuntu (or at least non 10.04) systems soon. Most of the other systems I have access to don't have valgrind installed.

Ryan Budney 2010-08-25 07:29:48

@Ryan: Thanks for accepting this answer… you can also answer it yourself and select that, to avoid a low acceptance rate.

Potatoswatter 2010-08-27 20:25:55

Your comments have been the most helpful, and helped me identify the source of the problem -- valgrind's general over-exuberance. I'd give points to Starkey, too, if I could.

Ryan Budney 2010-08-31 01:36:11

ansaurus

tags:

views:

answers:

A very simple thread pool using pthreads in C++

related questions