1

Below is my C code to print an increasing global counter, one increment per thread. 

#include <stdio.h>
#include <pthread.h>

static pthread_mutex_t pt_lock = PTHREAD_MUTEX_INITIALIZER;
int count = 0;

int *printnum(int *num) {
    pthread_mutex_lock(&pt_lock);
    printf("thread:%d ", *num);
    pthread_mutex_unlock(&pt_lock);
    return NULL;
}

int main() {
    int i, *ret;
    pthread_t pta[10];
    for(i = 0; i < 10; i++) {
        pthread_mutex_lock(&pt_lock);
        count++;
        pthread_mutex_unlock(&pt_lock);
        pthread_create(&pta[i], NULL, (void *(*)(void *))printnum, &count);
    }
    for(i = 0; i < 10; i++) {
        pthread_join(pta[i], (void **)&ret);
    }
}

I want each thread to print one increment of the global counter but they miss increments and sometimes access same values of global counter from two threads. How can I make threads access the global counter sequentially?

Sample Output:

thread:2 thread:3 thread:5 thread:6 thread:7 thread:7 thread:8 thread:9 thread:10 thread:10

Edit

Blue Moon's answer solves this question. Alternative approach is available in MartinJames'es comment.

Samik
  • 575
  • 2
  • 8
  • 19
  • 3
    Well, yes, because you are passing the address of the one counter to threads that are running asynchronously to the loop incing the counter. malloc an int, load the int from the counter and pass the address of the new int to the thread. Free the int* in the thread before it terminates to prevent leaks. – Martin James Jun 25 '15 at 20:25
  • Whatever platform you're on most likely declares atomic increment and decrement functions. See OSAtomicIncrement on OSX, InterlockedIncrement on Windows, __sync_fetch_and_add in gcc, etc. – i_am_jorf Jun 25 '15 at 20:28
  • Do you only want the threads to *print* each increment? Why not have each thread incrementing the counter? – Filipe Gonçalves Jun 25 '15 at 20:29
  • 1
    @i_am_jorf That is not the issue here. Even with an atomic increment variable, the threads would be racing with the main thread to read the count. It may happen that 3 or 4 threads all read the same value before the main thread has the chance to increment it again. Plus, he's handling concurrency with a mutex. – Filipe Gonçalves Jun 25 '15 at 20:30
  • I don't understand what you're trying to achieve: is this an exercise to learn IPC between threads? You can use some sort of communication between the threads so that each thread knows when to read `count`. Or you can just dynamically allocate a new `int` with the current value of `count` and pass it to each thread. – Filipe Gonçalves Jun 25 '15 at 20:32
  • Casting a function pointer is a bad idea. Calling a function not according to its prototype is an error that leads to undefined behavior, pointers may have different representation and calling conventions can be different according to the argument type. Generally casts are evil, you pretend to know better than the compiler. Here you'd just have to declare `printnum` with the correct signature and then have `int* num = arg` to *convert* (not cast) the argument to the type that you are expecting. – Jens Gustedt Jun 25 '15 at 20:36
  • 1
    @FilipeGonçalves this code is to clarify my understanding about pthreads, actually I was trying to make a multi-threaded character occurence counter where each thread should search sequencial portions of a seekable file and return the count to main thread via join. This requires me to send a structure to the start routine containing filename, start, length etc. I think MartinJames'es approach would be better here? – Samik Jun 25 '15 at 21:04
  • @Samik Yes, MartinJames' approach seems a reasonable choice. You would allocate the structure dynamically and free it in the thread once it's done. If you run into problems, post a new question: you will probably get better answers targeted at your specific code. In my opinion this question sounds a bit weird because it's hard to understand what's the use of this code. – Filipe Gonçalves Jun 25 '15 at 21:12

1 Answers1

2

A simple-but-useless approach is to ensure thread1 prints 1, thread2 prints 2 and so on is to put join the thread immmediately:

pthread_create(&pta[i], NULL, printnum, &count);
pthread_join(pta[i], (void **)&ret);

But this totally defeats the purpose of multi-threading because only one can make any progress at a time.

Note that I removed the superfluous casts and also the thread function takes a void * argument.

A saner approach would be to pass the loop counter i by value so that each thread would print different value and you would see threading in action i.e. the numbers 1-10 could be printed in any order and also each thread would print a unique value.

P.P
  • 117,907
  • 20
  • 175
  • 238
  • 2
    [Nitpick] passing the loop counter by value is not really possible. We must go through the hassle of dynamically allocating an `int`, storing the counter value in there, pass the pointer, and then free it before the thread exits. Of course, we can get sloppy and cast the counter value to a pointer and try to cast it back to `int` in the thread, but that is ugly and not guaranteed to work. – Filipe Gonçalves Jun 25 '15 at 20:47
  • the signature of `pthread_create` is preventing me to pass the argument to the start routine by value, it's a `void*`. – Samik Jun 25 '15 at 20:50
  • 1
    @FilipeGonçalves Not really true. There's `intptr_t` for this purpose. – P.P Jun 25 '15 at 20:51
  • @Samik You can call: `pthread_create(&pta[i], NULL, printnum, (void*)i);` and retrieve it back: `int i= (intptr_t) num;`. – P.P Jun 25 '15 at 20:53
  • Wow, didn't know about this one, but I guess that works only with integer and shorter datatypes right? – Samik Jun 25 '15 at 20:57
  • @Samik It would work for any integer type. But a downside is `intptr_t` is it's an optional type. But I am not sure of any mainstream compiler that doesn't provide it. – P.P Jun 25 '15 at 21:00
  • @BlueMoon Except that using intptr_t family for *anything else* than storing a pointer is undefined by C Standard. – this Jun 25 '15 at 21:11
  • @this Can you quote the standard where it says it's undefined? – P.P Jun 25 '15 at 21:14
  • @BlueMoon You want me to quote something that isn't defined?? If Standard imposes no requirements for something then it is undefined *3.4.3, p1*. Defined behavior requires positive evidence, therefore if you want to argue that something *is defined*, it is you who must quote the Standard. – this Jun 25 '15 at 21:52
  • @this First of all, the standard relevant here is POSIX. Pthreads is not in ISO C either. The C standard, as you said, imposes no requirements on things outside of its definition. That means, if X is undefined in ISO C then a particular implementation can define its behaviour and guarantee it to always work in a certain way. It's just that it's not ISO C conforming. Re: intptr_t is defined as *The following type designates a signed integer type with the property that any valid pointer to void can be converted to this type, [...]* – P.P Jun 25 '15 at 22:22
  • @this *[...] then converted back to pointer to void, and the result will compare equal to the original pointer* which says `void*` => `intptr_t` => `void *` is well defined. And 6.3.2.3 states *An integer may be converted to any pointer type. Except as previously specified, the result is implementation-defined, might not be correctly aligned, might not point to an entity of the referenced type, and might be a trap representation. 67)* which allows converting an `int` to `void *`. – P.P Jun 25 '15 at 22:24
  • @this While this doesn't explicitly guarantee, it's left to implementation. So at worst, if `int` is wider than `intptr_t` then there may be loss of information. But it's nowhere close to undefined. – P.P Jun 25 '15 at 22:24
  • @BlueMoon The last quote you posted lists implementation defined behavior with specific caveats that result in undefined behavior. Therefore the behavior is undefined unless the implementation just happens to handle this specific case completely to avoid undefined behavior. You using the phrase "nowhere close" for this example is simply disingenuous. – this Jun 25 '15 at 23:01
  • @this It's not "three" different categories but single one. The trap and alignement issue is applicable when using it as a pointer, not just for value conversion. Your assertion that "it is undefined because I state so" is what's really disingenuous. Care to point to a POSIX implementation where this conversion fails? – P.P Jun 25 '15 at 23:13
  • @bluemoon: the conversion works on every platform I use, but it is not *guaranteed* to work. If intptr_t exists, then any pointer can be converted to intptr_t but the reverse is not guaranteed. If you want ironclad, create an array with enough elements and pass `(void*)(array + n)`. In this case you could use the `pta` array – rici Jun 25 '15 at 23:40
  • 1
    @bluemoon: and "this" is corrrect. Converting an integer to a void* is undefined unless explicitly allowed. See conversions in the standard. Explicitly allowed are 0 and an intptr_t previously converted from a pointer. "1" doesn't fall into either category. (But I agree that it is a purely theoretical concern.) – rici Jun 25 '15 at 23:56
  • @rici That could be overcome with: `(void*)(intptr_t)i` in passing and `int i = (int)(intptr_t)arg;` in retrieving. Again I am not claiming this is the best solution or portable. Am just not convinced it's undefined. – P.P Jun 26 '15 at 03:41
  • 1
    intptr_t is still an integer type, which means that casting an intptr_t to a void* is allowed to be a trap representation, and passing a trap representation to a function is allowed to trap. That strikes me as an unlikely scenario, but it might happen in theory: Say pointers are segment/offset and segment 0 is illegal in non-privileged mode. An implementation is not required to check if a void* is a trap representation, but it's possible to imagine a "safe mode" in which this check occurs. Having said all that, it's possibly not quite in the same category as signed integer overflow, but close. – rici Jun 26 '15 at 04:33
  • I dug the POSIX standard and I couldn't find a reference either way. So the best I can conclude is if the result can be represented (per C11) then it's implementation-defined, otherwise undefined. There's no implementation of posix where this fails that I am aware of (AIX, Linux, Solaris, SCO etc). Categorically saying it's undefined based on some archaic clauses that C standard had to accommodate is not correct. Even then C standard allows it to be implementation-defined. – P.P Jun 26 '15 at 08:02
  • @P.P.: You don't need to join (reap) the threads; you can also use a semaphore to synchronize the thread creation and accessing the index variable passed via pointer to the thread function. Example code [in this related answer](https://stackoverflow.com/a/52470556/1475978). – Nominal Animal Sep 24 '18 at 16:30
  • 1
    Also, the comments above whether passing an int via a pointer type to the thread worker is portable or not is completely academic: there exists a lot of C/POSIX code that expects that to work, so any POSIXy implementation (noting that most POSIXy ones are not certified as being POSIX) will either ensure that works, or it will not be able to run code that works fine on basically all other POSIXy systems. Practice trumps theory, even standards. – Nominal Animal Sep 24 '18 at 16:33
  • @NominalAnimal Indeed that's another way. Your answer is rather long (not complaining), so I'll read it later when I have time. – P.P Sep 24 '18 at 16:35
  • @P.P.: No worries! (I got hung up on the assertion *"The only way ..."* at the beginning of your answer. I would not object to *"simplest"* or *"best"* or other qualifiers, it's just that *"only"* is misleading, IMHO.) – Nominal Animal Sep 24 '18 at 16:48
  • @NominalAnimal I have corrected the wording. I fully agree that the int->ptr->int ismainly academic (I was downvoted commenting that even though it's not even a part of my answer!). Your idea of semaphore is slightly better but not quite what I'd like (I just think it's an unnecessary co-ordination when both threads could continue if an aux array could be used). – P.P Sep 24 '18 at 17:39
  • @P.P. Fully agreed. Integer-as-pointer-payload is common, but not what I personally recommend. I prefer the pool approach, as it scales well, works well with differing work/job lengths, and is usually easier to implement correctly. My answer there is long, because I (am overly verbose and) needed the two steps in between to get to the pool solution I recommend, without skipping any of the reasons. Definitely could use better wording, as me fail English often. – Nominal Animal Sep 24 '18 at 18:14