pithikos/c-thread-pool

A minimal but powerful thread pool in ANSI C

c-thread-pool's Issues

How to keep track of number of running threads?

In may be a request for enhancement rather than an issue, but , is there a way to keep track of number of active threads at each time?

X= thpool->num_threads_working; gives me the following error:

Myprog.c:872:40: error: dereferencing pointer to incomplete type ‘struct thpool_’
X=thpool->num_threads_working;

Thanks,

getting error cannot convert to a pointer type when passing a struct as args

my code requires me to pass a struct to task for the threads and i get this error
convert to a pointer type

My code is

struct Tuple {
int a;
int b;
};
void searchTree(struct Tuple exvals)

{
int e1 ,e2;
e1 = exvals.a;
e2 = exvals.b;
..............
}
void main(int argc, char *argv[])
{
struct Tuple expandVals;
expandVals = searchANode(currnode);//another function

thpool = thpool_init(numThreads);
thpool_add_work(thpool, (void_)searchTree, (void_)expandVals);

puts("Killing threadpool");
thpool_destroy(thpool);

}

unconditional (forced) destroy of thread pool

Hello,
Is there an API to destroy a pool without waiting for executing threads to complete?
Or is it easy to create one based on the current destroy API ? (thpool_destroy)
Thanks.

support win32

First of all, thanks so much for making this project available - there is nothing quite like this out there for c language.

Have you thought about supporting windows ? I would be happy to work on this, as I need it for one of my projects.

Cheers,
Aaron

Do not quite understand example.c - lines 37&38

Hi,

Maybe it's not really an issue but I can't find anywhere else to ask… I read thpool.c and it seems that the 2nd argument of thpool_add_work is a "function pointer", but in example.c, lines 37&38, the type of 2nd argument is explicitly (void*).

I was not familiar with that, but on StackOverflow I found a question Why can't I cast a function pointer to (void *)?, which might be related to this case (although I'm not quite sure whether this case is the so-called casting).

By the way, adding -Wpedantic does give warnings when I compile example.c. Either removing (void*) or replacing it with (void (*)()) will make the warnings disappear (although other warnings exist).

One poniter position is wrong.

thpool.c line 285

    *thread_p = (struct thread*)malloc(sizeof(struct thread));
	if (thread_p == NULL){
		err("thread_init(): Could not allocate memory for thread\n");
		return -1;
	}
///should change to next
*thread_p = (struct thread*)malloc(sizeof(struct thread));
	if (*thread_p == NULL){
		err("thread_init(): Could not allocate memory for thread\n");
		return -1;
	}```

Problem at initlize thpool with num = 0

I just missing around with clang analyzer

using

clang --analyze -Xanalyzer -analyzer-output=text thpool.c

thpool.c:140:39: warning: Call to 'malloc' has an allocation size of 0 bytes
        thpool_p->threads = (struct thread**)malloc(num_threads * sizeof(struct thread *));

Hope to Help

memory keep Increasing -solved

It turned out that my own code had memory leakage. After fixing it, everything works well.

Sorry, I don't know how to delete this issue and I can't close it myself. ==|||

Resuming a threadpool resumes all

At the moment pause/resume synchronisation commands affect all thread pools. Someone issuing thpool_pause(thpool) will cause all thread pools to pause for example even if only one thread pool is passed as argument.

To work around this, all thread specific variables should be bundled inside the thread pool structure.

(initial report: #11)

Malloc-ing more than needed memory

the code in thpool.c at 143 lines is as blow:
thpool_p->threads = (struct thread**)malloc(num_threads * sizeof(struct thread));

i think it shuold be
thpool_p->threads = (struct thread**)malloc(num_threads * sizeof(struct thread *));

Recursive calls

I want to create a merge sort using your library, but from my brief reading of your work, you are using a LIFO way of processing workload, which in case of merge sort is breaking the flow of recursion.
Is there a way to pause all previous called threads and run a new one, or make my program force the most recent work to be processed first ?

Additional function that allows passing pthread_attr_t during thread pool initialization

Hello,

First of all, I'd like to say thank you for your library.
I want to propose extension of current thread pool initialization process in order to be able to pass pthread attribute to underneath threads which are created in a pool.
It might be done via additional method like:

const pthread_attr_t *internal_pattr_variable = NULL;
//...
void thpool_set_attr(const pthread_attr_t *pattr)
{
    internal_pattr_variable = pattr;
}

By doing so we can check whether our internal_pattr_variable is not NULL in the thread_init call and if so, apply the provided attributes to the brand new threads.

If you do not mind I can do that and send a pull request.

save trained model with non-C++11

Hi patrikhuber,

I noticed that you used C++11 lib cereal to serialize the trained model and save it to binary file.
you use the following code to save the model:
void save_detection_model(detection_model model, std::string filename)
{
std::ofstream file(filename, std::ios::binary);
cereal::BinaryOutputArchive output_archive(file);
output_archive(model);
};

My question is:
do you have any other substitute solution that doesn't use C++11 syntax about how to save/load the model to/from files?
can you share non-C++11 code about how to save/load the model?

work distribution in differents thread based on hash

I would like to allocate some specific work to some specific thread based on some criteria.

So for this need, I plan to extend C-Thread-Pool to support multiple queues. Perhaps, one queue by worker thread is the right approach. And the new job will be distributed to the right queue based on hash computation in my application.

Could you provide some guidance about the design? I would like to contribute and I want to do it in a clean way with a clean design

Should we destroy mutex and cond?

Hi,
There is

pthread_mutex_init(&(thpool_p->thcount_lock), NULL);
pthread_cond_init(&thpool_p->threads_all_idle, NULL);

But I can't find pthread_mutex_destroy and pthread_cond_destroy call,
should those two functions be called at thpool_destroy()?

Thanks.

No synchronization with main thread.

Interesting library, and would use this, but:

It seems to be missing important synchronization functionality.
There is no mechanism provided that will let the main thread go to sleep until a job queue is empty.

A typical use in multi-threaded programming would be:

• kick off N threads to do the work
• wait for work completion (without burning CPU cycles caused by spinning.)
• integrate the results by assembling the pieces computed in worker threads.

Such a mechanism could be provided by pthread_barrier_t but unfortunately, this is not part of POSIX, and is lacking in OSX.

Fix for jobqueue_init

Hi,
Very nice work ! Thanks :-)

May I suggest 2 little fixes for jobqueue_init :

static int jobqueue_init(thpool_* thpool_p){

thpool_p->jobqueue_p = (struct jobqueue*)malloc(sizeof(struct jobqueue));
if (thpool_p->jobqueue_p == NULL){
    return -1;
}

// fix1 : pthread_mutex_init should be called after testing thpool_p->jobqueue_p
pthread_mutex_init(&(thpool_p->jobqueue_p->rwmutex), NULL);

// fix2 : thpool_p->jobqueue_p->len should be initialized because it's used by jobqueue_clear (the condition in the while loop)
thpool_p->jobqueue_p->len=0 ;

thpool_p->jobqueue_p->has_jobs = (struct bsem*)malloc(sizeof(struct bsem));
if (thpool_p->jobqueue_p->has_jobs == NULL){
    return -1;
}
bsem_init(thpool_p->jobqueue_p->has_jobs, 0);

jobqueue_clear(thpool_p);
return 0;

}

why need pause and resume?

Hi,
I see those two api in tests, and if I add work from client requests through TCP like below,
(implemented in main thread)
switch ... { case 1: thpool_add_work(thpool, (void *)get_time, (void *)local); case 2: thpool_add_work(thpool, (void *)get_file, (void *)local);

In this case, do I need paUse and resume to surround those add works?
If these needed, how to ? and sleep also needed too ?

Bug when compiled with optimization?

If you compile thpool.c with optimization it seems to hang.

To see this, edit tests/threadpool and change the line
gcc src/conc_increment.c ../thpool.c -pthread -o test
to
gcc -g -O src/conc_increment.c ../thpool.c -pthread -o test
Then when you run ./threadpool, it hangs at

$ gdb -p 22284 ./test
0x00000000004011c5 in thpool_init (num_threads=4) at ../thpool.c:150
150 while (thpool_p->num_threads_alive != num_threads) {}
(gdb) p thpool_p->num_threads_alive
$1 = 4
(gdb) p num_threads
$2 = 4

This is with gcc-4.8.3.

If I change thpool.c, adding "volatile",

typedef struct thpool_{
    thread**   threads;                  /* pointer to threads        */
    volatile int num_threads_alive;        /* threads currently alive   */
    volatile int num_threads_working;      /* threads currently working */
    pthread_mutex_t  thcount_lock;       /* used for thread count etc */
    jobqueue*  jobqueue_p;               /* pointer to the job queue  */    
} thpool_;

That seems to fix it, though it might be worth compiling the other tests with optimization to see if everything still works ok.

Segmentation fault (core dumped)

Hi,

I use your code to accelerate my calculations. I created a thpool with 4 workers and added 50 threads to it. When I ran my code, it reported "Segmentation fault (core dumped)". I don't know why.

I ran your example.c correctly. Perhaps each thread uses too much memory. I use each thread to compute a complex cost function which handles arrays of millions of double numbers. My laptop has 8G memory; hence, I think I can create 4 workers to compute some cost functions of some parameters concurrently. I tested that it took 0.7s to compute the cost function. Any advice? Thank you!

Completion of error handling

Would you like to add more error handling for return values from functions like the following?

• pthread_create ⇒ thread_init
• pthread_mutex_init ⇒ thpool_init

The queue is growing as big as the memory!

I have created a producer and several comsumer threads.
Producer add work to the threads using thread_add_work function.
Consumers threadpool threads do their job.

Problem is:
if number of thread is 5 and producer is 100 times faster than the consumer, then queue size is just increasing.

'SIGUSR1' undeclared

I get a couple of errors when trying to compile thpool.c

-- Configuring done
-- Generating done
-- Build files have been written to: D:/msys64/home/Projects/test/bin
D:/msys64/home/Projects/test/src/lib/thpool.c: In function 'thpool_pause':
D:/msys64/home/Projects/test/src/lib/thpool.c:249:47: error: 'SIGUSR1' undeclared (first use in this function)
  249 |   pthread_kill(thpool_p->threads[n]->pthread, SIGUSR1);
      |                                               ^~~~~~~
D:/msys64/home/Projects/test/src/lib/thpool.c:249:47: note: each undeclared identifier is reported only once for each function it appears in
D:/msys64/home/Projects/test/src/lib/thpool.c: In function 'thread_do':
D:/msys64/home/Projects/test/src/lib/thpool.c:336:19: error: storage size of 'act' isn't known
  336 |  struct sigaction act;
      |                   ^~~
D:/msys64/home/Projects/test/src/lib/thpool.c:337:2: warning: implicit declaration of function 'sigemptyset' [-Wimplicit-function-declaration]
  337 |  sigemptyset(&act.sa_mask);
      |  ^~~~~~~~~~~
D:/msys64/home/Projects/test/src/lib/thpool.c:340:6: warning: implicit declaration of function 'sigaction' [-Wimplicit-function-declaration]
  340 |  if (sigaction(SIGUSR1, &act, NULL) == -1) {
      |      ^~~~~~~~~
D:/msys64/home/Projects/test/src/lib/thpool.c:340:16: error: 'SIGUSR1' undeclared (first use in this function)
  340 |  if (sigaction(SIGUSR1, &act, NULL) == -1) {
      |                ^~~~~~~
D:/msys64/home/Projects/test/src/lib/thpool.c:336:19: warning: unused variable 'act' [-Wunused-variable]
  336 |  struct sigaction act;
      |                   ^~~
make[2]: *** [CMakeFiles/test.dir/build.make:5491: CMakeFiles/test.dir/src/lib/thpool.c.obj] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/Makefile2:125: CMakeFiles/test.dir/all] Error 2
make: *** [Makefile:104: all] Error 2
 [ 0%] Building C object CMakeFiles/test.dir/src/lib/thpool.c.obj

Any idea how to fix those?

My variables is not change

the problem is that I defined a struct to store the variable i changed in a foop loop, but after I added all works, then I use function thpool_wait() to wait the work done, but the result is not as expected, the result should be a list of numbers from 0-99, maybe the sequence is variant, but the result of mine is always 99, hope your help,thanks
the code
`#include "thpool.h"

struct param
{
int i;
};
void subfunc(void *arg)
{
struct param *p = (struct param *)arg;
printf("i = %d\n", p->i);
}
int main()
{
threadpool tp = thpool_init(4);
int i = 0;
struct param p;
for (i = 0; i < 100; i++)
{
p.i = i;
thpool_add_work(tp, subfunc, (void *)&p);
}
thpool_wait(tp);
thpool_destroy(tp);
}`

the result as follow
~/Code/1010$ ./test.o i = 3 i = 99 i = 99 i = 99 i = 99 i = 99 i = 99 i = 99 i = 99 i = 99 i = 99 i = 99 i = 99 i = 99 i = 99 i = 99

function type for the worker task

task are of type

void *(*function_p)(void*)

http://cdecl.ridiculousfish.com/?q=void+*%28*function_p%29%28void*%29

where I believe they should be

void (*function_p)(void*)

http://cdecl.ridiculousfish.com/?q=void+%28*function_p%29%28void*%29

I believe that I causes a bus error when you try to put the first type on the pool and the manager would try to pull them from the pool, plus there is no way to return whatever the worker return, except via output parameters

Unresolved conflict

C-Thread-Pool/thpool.c

Line 459 in 430251c

<<<<<<< HEAD

Should theadpool create and destroy threads dynamically?

Current threadpool in this project creates threads statically when it is initialized. And number of threads is fixed during its lifecycle.

Why not create and destroy threads dynamically?

Create:

• case 1: number of current threads is less than number of core threads.
• case 2: number of current threads is less than number of max threads, and no idle threads exist now.

Destroy:

• thread has waited for keep_alive_time time interval without jobs fetched.
• core threads can ignore keep_alive_time and keep alive if allowed.

incomplete definition of type 'struct thpool_'

The usage shown in the readme (compiling thpool.c along with myapp.c) is fine for building applications, but fails for the case of generating the .o for my_library_component.c, which has a struct thpool_* variable inside.

I presume the reason for hiding the definition of struct thpool_ is that, in order to expose it, you'd have to expose some of the other structs as well, and you want to keep those definitions private. But this seems to result in some pretty awkward build semantics. Wouldn't it make sense to just expose those structs, so thpool.c could just be another .o, as we build our own library that uses C-Thread-Pool internally?

Or, alternatively, threadpool could just be an opaque (i.e. void*) handle.

question about threads_keepalive variable.

Hi, thanks for this awesome lib.

In thpool.c (line 37), here is static volatile int threads_keepalive; , why not put threads_keepalive on struct thpool_?

Assume the following scenario:

1. Create thread pool A (Now threads_keepalive is set to 1);
2. Call add_task on thread pool A;
3. Create thread pool B (Now threads_keepalive is set to 1);
4. Call add_task on thread pool B;
5. Destroy thread pool A (Now threads_keepalive is set to 0);
6. Call add_task on thread pool B ; ( task won't execute, because of threads_keepalive == 0 )

How to use this with C++ code?

I was trying to figure out how to use this threadpool for my C++ program? Mainly compiling and linking it. I see that there is an 'extern C' for if we have C++ code, but wondering how to use it? Currently these are the errors I get when trying:

thpool.c:86:12: error: declaration of ‘jobqueue thpool_::jobqueue’ [-fpermissive]
jobqueue jobqueue; /* job queue /
^
thpool.c:68:3: error: changes meaning of ‘jobqueue’ from ‘typedef struct jobqueue jobqueue’ [-fpermissive]
} jobqueue;
^
thpool.c: In function ‘int thread_init(thpool_, thread**, int)’:
thpool.c:293:76: error: invalid conversion from ‘void*’ to ‘void* ()(void)’ [-fpermissive]
pthread_create(&(thread_p)->pthread, NULL, (void )thread_do, (thread_p));
^
In file included from thpool.c:16:0:
/usr/include/pthread.h:244:12: note: initializing argument 3 of ‘int pthread_create(pthread_t, const pthr ead_attr_t, void ()(void), void*)’
extern int pthread_create (pthread_t *__restrict __newthread,

I'm trying to compile/link like this:

g++ -std=c++11 main.cpp thpool.c -o main -lssh -pthread

But it doesn't work. How would I make this work?

License

Could you please include a license for the project?

Common Pointers for all the jobs in the queue.

In the function thpool_add_work of thpool.c the assignmenet of the newJob is as follows:
newJob->arg=arg_p
where arg_p is the argument of the function.
This leads to a situation in which all the jobs in the queue having the same pointer.
This can be mitigated by replacing the lines with:

newJob->arg=(char_)malloc(10000_sizeof(char));
strcpy(newJob->arg,arg_p);

but this way we lose the obscureness of the API.

Errors when compiling with C++

I've found your threadpool incredible and am using it for my C++ project. However, when I built it with g++ and c++11 library (option -std=c++11 ), i got into this issue:

thpool.c:68:3: error: changes meaning of ‘jobqueue’ from ‘typedef struct jobqueue jobqueue’ [-fpermissive] } jobqueue;

I've searched for solution and found out that princessannabelle had issued this on 2017. However, the problems may not lie in the worker function, but in some structure names that you've defined in thpool.cpp. Some of them are the same as some of your varriables, so that g++ got confused and raised errors, which may not happen with gcc due to differences in privacy of the 2 compilers.

I tried to change all the duplicated names and things works fine.

So, i opened this issue and suggest some modifications so that both C and C++ users can enjoy your work with more convenient. Here are my changes on your code that flow fine.

Thanks for your consideration

Add threads to thread pool

Hi All,

I am thinking a feature which can dynamically add threads to thread pool.

We can call it thpool_add_thread(int num);

How do you think about this proposal?

Thanks!
hdl

Some Issues need to be helped！

`#include <stdio.h>
#include <pthread.h>
#include "thpool.h"

int c1 = 0;
int count = 0;
void task1(){
int i;
for (i = 0;i < 1000;i++)
count++;
printf("%d Thread %d working on task1\n",count ,c1++);
}

void task2(){
int i;
for (i = 0;i < 1000;i++)
count++;
printf("%dThread %d working on task2\n",count, c1++);
}
int main(){
puts("Making threadpool with 4 threads");
threadpool thpool = thpool_init(4);

puts("Adding 40 tasks to threadpool");
int i;
for (i=0; i<20; i++){
	thpool_add_work(thpool, (void*)task1, NULL);
	thpool_add_work(thpool, (void*)task2, NULL);
};
thpool_wait(thpool);
puts("Killing threadpool");
thpool_destroy(thpool);

return 0;

}`
the result of this file is variant, I don't know what is going on about this, can someone help me solve this problem? Thanks!

Mac Os Warning: implicit declaration of function 'pthread_setname_np'

Compiling thpool on Mac Os with lldb 370.0.42 triggers the above message.

cc -Wall -D_GNU_SOURCE -std=gnu99 -O2 -g  -c -o dep/thpool/thpool.o dep/thpool/thpool.c
dep/thpool/thpool.c:298:3: warning: implicit declaration of function 'pthread_setname_np' is invalid in C99
      [-Wimplicit-function-declaration]
  pthread_setname_np(thread_name);
  ^

Removing

#define _POSIX_C_SOURCE 200809L

which was added in a391602 gets rid of the warning, and passes on Linux as well.

Any hint on how to fix this warning? Is it safe to just comment out the _POSIX_C_SOURCE line?

Add more job than threads

if much job are added into queue, and now the threads are busy , no thread is waiting the mutex, so if the threads finish the work, it will not be notified again.

Any question weclomed in https://github.com/52coder/C-Thread-Pool

Since the author is no longer maintained, if you have any questions, please discuss with me in this repository
https://github.com/52coder/C-Thread-Pool

I have used this project in the actual project. If there is any problem, please feedback, I will fix it as soon as possible

How can i compile using header file

I have three files : main.c, functions.c and functions.h, can i compile using something like this "gcc main.c functions.c thpool.c -D THPOOL_DEBUG -pthread -o main"?

want to add multiple tasks to thread pool using same function with different args

hi,i want to do smt like this

  for (i=0; i<2; i++){
                j = snprintf(fn,sizeof(fn),"./xa%c",'a'+i); 

                fn[j] = '\0';

                printf("fn=%s\n",fn);

                thpool_add_work(thpool, (void*)task, (void*)fn);

        };  

fn is file name,but it seems two tasks in thread pool deal with the second fn,that is ./xab

Why polling in thpool_wait?

The exponential polling is a clever way to not waste CPU too much, but why not wait on a condition var (e.g. jobs_empty) that's essentially the opposite of has_jobs? That would eliminate the need for polling at all.

Improve signal handler

The variable "threads_on_hold" should use the data type "sig_atomic_t", shouldn't it?

thpool.c has leftovers after some merge

At line 459 inside thpool.c, we can see there is some forgotten leftovers after some merge.

/* Get first job from queue(removes it from queue)
<<<<<<< HEAD
 *
 * Notice: Caller MUST hold a mutex
=======
>>>>>>> da2c0fe45e43ce0937f272c8cd2704bdc0afb490
 */

It seems to be contained inside /* */ so that is why it may have been unnoticed till now (code compiles just fine with this being in).

merge conflict in comment at thpool.c:459

Looks like there's some kind of accidental merge conflict in a comment:

C-Thread-Pool/thpool.c

Line 459 in 430251c

<<<<<<< HEAD

/* Get first job from queue(removes it from queue)
<<<<<<< HEAD
 *
 * Notice: Caller MUST hold a mutex
=======
>>>>>>> da2c0fe45e43ce0937f272c8cd2704bdc0afb490
 */

Remove exit(1)

There are a bunch of exit(1) that occur in the code. This is a bad design for a library since it takes control away from the user.

Instead of exit(1) returning specific error codes should be used.

Is this library maintained?

There are numerous years-old pull requests, the library appears to have been left to rot. That would be a shame because I haven't been able to find any other good pure-C threadpool libraries.

Error Handling

Add error handling to possible failure points by returning an error code, and document any that we use for better control by the calling application.

Doesn't release memory until job queue is empty?

I have an application using Thread Pool that will often result in job queues of more than 25,000 jobs. This is expected and the delay in processing the jobs isn't a problem. However, the memory consumed by the queue doesn't seem to be released once the job is executed. At peak the resident memory of the application might reach 1GB when the job queue is peaking. However, when the job_queue reaches zero the resident memory is largely cleared.

I've run everything through valgrind multiple times and it's not indicating any memory leaks when the application shuts down gracefully. When I put in a dummy call back that keeps the queue at a minimum size the memory consumption is pretty low. The only thing that seems related to the total memory consumption is the job queue length. That the resident memory clears once the queue is empty is odd to me. Do you have any suggestions?

Thanks

thpool_init illogical

struct thpool_* thpool_init(int num_threads){

	threads_on_hold   = 0;
	threads_keepalive = 1;

-	if (num_threads < 0){
-		num_threads = 0;
-	}
+	if (num_threads < 1){
+		num_threads = 1;
+      // OR assert(...)
+	}

// jump to -> 
thpool_p->threads = (struct thread**)malloc(num_threads * sizeof(struct thread *));
// -> 0 x N = 0; -> malloc(0);
// anyhow what would be the point of having a thread pool without any thread ? 8-)

thusss.

Dead locks

Hi Guys

Great idea to make a simple ANSI-C threadpool. The interface is great, but I have encountered numerous problems using it.

In your example:

After adding the work

for (i=0; i<20; i++){
thpool_add_work(thpool, (void*)task1, NULL);
thpool_add_work(thpool, (void*)task2, NULL);
};

if you add any of the following, you get a dead lock

thpool_pause(thpool);
thpool_resume(thpoll);

or

thpool_pause(thpool);
printf("nthreads alive: %d\n", thpool_num_threads_working(thpool));
sleep(1);
thpool_resume(thpool);

Accessing the variable num_threads_working is causing the error. Volatile does not imply atomic updates.

Another issue is all conditions and mutex'es. You are not allowed to reinitialize any of the two. This is possible in many ways. As long as only a single queue is supported, I can recommend using PTHREAD_COND_INITIALIZER and PTHREAD_MUTEX_INITIALIZER

I haven't been able to come up with a good solution supporting pause/resume.

Regards
Jens Munk