Returning std::pair, double> with phtread_exit

Question

I want to return a std::pair<std::vector<int>, double> from a thread.

The setup is the following:

void* thread_run(void *);
int main(int argc, char* argv[]) {

     std::pair<std::vector<int>, double> value;

     pthread_t t;
     pthread_create(&t, NULL, thread_run, NULL);

     // wait
     void* out;
     pthread_join(t, &out);

     value = *(std::pair<std::vector<int>, double> *)out;
     free(out);

     use(value);
}

void* thread_run(void* arg) {
    std::pair<std::vector<int>, double> * out = (std::pair<std::vector<int>, double>*)
        malloc(sizeof(std::pair<std::vector<int>, double>));

    out->first  = calc1();
    out->second = calc2();

    pthread_exit(out);
}

The problem is that I create a memory leak. Valgrind reports that:

A conditional jump or move depends on unitialised values and points to the allocation out->first = calc1();
Unitialized value was created by a heap allocation and points to the malloc line.

I am using gcc 5.4.0 & C++ 11 and the pthread library. I need to use pthread. How do I return C++ STL containers correctly to avoid the memory leak.

My understanding is that there's no cancellation mechanism for std::thread. I need a cancellation mechanism. Further, if going the malloc route, would I have to give up on returning std containers from the thread? — ragnacode, Nov 23 '19 at 20:27
You should giving up on creating _any_ C++ object with malloc. — tkausl, Nov 23 '19 at 20:28
Use `std::thread`. For cancellation mechanisms, do `pthread_cancel(std::thread::native_handle())` [Here is a example](https://stackoverflow.com/questions/13531241/cancelling-stdthread-using-native-handle-pthread-cancel) — KamilCuk, Nov 23 '19 at 20:28
Thank you all for your valuable input, I can only select one answer but all of your feedback has been very valuable. I wish there was a way in SO to reward everyone who contributed. — ragnacode, Nov 23 '19 at 20:40

walnut · Answer 1 · 2019-11-23T20:34:36.860

Your program has undefined behavior, because you allocate memory for out, but never construct an object in this memory.

Then with out->first = calc1(); you try to access a member of the non-existent object, which causes undefined behavior.

If you need to manually manage memory, then you need to use new:

auto out = new std::pair<std::vector<int>, double>{};

and later delete it with

delete static_cast<std::pair<std::vector<int>, double>*>(out);

Note that you need to delete the pointer with the same type that it was created with. You might want to do the cast immediately when returning.

In C++ using malloc is almost always wrong and you should be using std::thread which comes with a proper portable C++ interface, instead of the POSIX-only C interface that pthread offers.

NicholasM · Answer 2 · 2019-11-23T20:35:57.370

How about this: don't allocate on the heap.

You successfully create a pair named value in main() with automatic storage duration. Just use that.

#include <vector>
#include <iostream>

#include <pthread.h>

void* thread_run(void *);

int main(int argc, char* argv[]) {
     std::pair<std::vector<int>, double> value;

     pthread_t t;
     pthread_create(&t, NULL, thread_run, &value);

     // Wait.  Do not need to retrieve results, which are available in `value`.
     pthread_join(t, nullptr);

     std::cout << "Results: " << value.first.at(1) << "; " << value.second << '\n';
}

void* thread_run(void* arg) {
    std::pair<std::vector<int>, double>& out = *static_cast<std::pair<std::vector<int>, double>*>(arg);

    out.first  = std::vector<int>{{1, 2, 3}};
    out.second = 42.0;

    pthread_exit(arg);
}

Runnable example on Coliru

KamilCuk · Accepted Answer · 2019-11-23T20:41:05.597

You just return a pointer to it, just like anything else. Don't use malloc, use new. Remember to call delete with proper type.

#include <vector>
#include <pthread.h>
#include <iostream>

void* thread_run(void *);
int main(int argc, char* argv[]) {

     std::pair<std::vector<int>, double> value;

     pthread_t t;
     pthread_create(&t, NULL, thread_run, NULL);

     // wait
     void* out;
     pthread_join(t, &out);

     std::pair<std::vector<int>, double> *pntvalue = 
        static_cast<std::pair<std::vector<int>, double>*>(out);
     value = *pntvalue;
     delete pntvalue;

    std::cout << std::get<1>(value) << std::endl;
}

void* thread_run(void* arg) {
    std::pair<std::vector<int>, double> *out = new std::pair<std::vector<int>, double>{};
    out->first  = std::vector<int>{1};
    out->second = 2;
    return out;
}

if going the malloc route, would I have to give up on returning std containers from the thread?

Not at all, but remember to properly call constructors and destructors of your objects. It would be something along:

#include <vector>
#include <pthread.h>
#include <iostream>

void* thread_run(void *);
int main(int argc, char* argv[]) {

     std::pair<std::vector<int>, double> value;

     pthread_t t;
     pthread_create(&t, NULL, thread_run, NULL);

     // wait
     void* out;
     pthread_join(t, &out);

     std::pair<std::vector<int>, double> *pntvalue = 
        static_cast<std::pair<std::vector<int>, double>*>(out);
     value = *pntvalue;
     pntvalue->~pair<std::vector<int>, double>();
     free(pntvalue);

    std::cout << std::get<1>(value) << std::endl;
}

void* thread_run(void* arg) {
    std::pair<std::vector<int>, double> *out = static_cast<
        std::pair<std::vector<int>, double> *>(malloc(sizeof(std::pair<std::vector<int>, double>)));
    new (out) std::pair<std::vector<int>, double>{};
    out->first  = std::vector<int>{1};
    out->second = 2;
    return out;
}

Returning std::pair, double> with phtread_exit

3 Answers3