pithikos / c-thread-pool Goto Github PK
View Code? Open in Web Editor NEWA minimal but powerful thread pool in ANSI C
License: MIT License
A minimal but powerful thread pool in ANSI C
License: MIT License
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.
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. ==|||
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.
The variable "threads_on_hold" should use the data type "sig_atomic_t", shouldn't it?
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?
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
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.
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,
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
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!
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;
}```
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.
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)
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.
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?
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
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 ?
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).
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
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.
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;
}
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
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
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.
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 ?
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.
Line 459 in 430251c
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.
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.
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 *));
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);
}
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).
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.
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.
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:
threads_keepalive
is set to 1);add_task
on thread pool A;threads_keepalive
is set to 1);add_task
on thread pool B;threads_keepalive
is set to 0);add_task
on thread pool B ; ( task won't execute, because of threads_keepalive == 0
)Looks like there's some kind of accidental merge conflict in a comment:
Line 459 in 430251c
/* Get first job from queue(removes it from queue)
<<<<<<< HEAD
*
* Notice: Caller MUST hold a mutex
=======
>>>>>>> da2c0fe45e43ce0937f272c8cd2704bdc0afb490
*/
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"?
Would you like to add more error handling for return values from functions like the following?
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
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.
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
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:
Such a mechanism could be provided by pthread_barrier_t but unfortunately, this is not part of POSIX, and is lacking in OSX.
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
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
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
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?
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?
`#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!
Could you please include a license for the project?
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.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.