unlocking mutex after returning object vector variable

Question

Some Background: I'm programming a simple taxi center project (c++, on linux), and I was asked to add multithreading in order to support some operations (I'm new to multithreading).

One of the operations I need to implement is calculating drivers path in other thread. In order to do that I want I created a class called PathCalculation, holds path vector and doneCalculation bool, so the thread that will calculate the path will receive pointer to the PathCalculation object, will set the vector.

the path vector is a local variable, and in the getPath method supposed to return it thread-safely, so I locked the vector's mutex, waited until the calculation was done using the conditional variable based wait, but when I was about to return the path vector,I encountered this problem: If I return the vector before unlocking the mutex, the mutex will never be unlocked. a possible solution is to unlock the mutex before returning the vector, but I am afraid it is going to create problems, with the threads:

Here is the code of the setPath and getPath:

void PathCalculation::setPath(const std::vector<Point> &path) {
    //lock the path vector and the done variable
    pthread_mutex_lock(&pmtx);
    done = true;
    _path = path;
    // signal that the path was calculated.
    pthread_cond_signal(&dcond);
    // Unlock the mutex
    pthread_mutex_unlock(&pmtx);
}

const std::vector<Point> &PathCalculation::getPath() const {
    // Lock the path vector mutex
    pthread_mutex_lock(&pmtx);
    //wait until the path will be setted
    while (!done)
        pthread_cond_wait(&dcond, &pmtx);

    //unlock the mutex here?
    return _path;
    pthread_mutex_unlock(&pmtx); //will be never executed...
}

My question is:

Is unlocking the mutex before returning the path vector might cause thread problems? And if it does, is there any thread-safe way to unlock the mutex and return the path vector?

Thanks, David

edit: since @Andrew Henle commented that some details are missing, I will try to explain more about the program:

Point class contains x,y. it is not thread safe, not designed to be thread safe. all I want to do is return the path in a thread-safe way. the class that is using the path vector is TaxiCenter: taxiCenter has function called addTrip(TripInfo* tripInfo), that before the multithreading update, toke that trip, calculated the path, constructed Trip object which contained tripInfo* and vector, and added this trip to vector that is called unassignedTrips. There is a class called Driver, which has setTrip(Trip* t) method, and the drive() method. when calling the drive method, the driver would "drive" by the trip it has, until reaching the end of the trip. to assign the trip, taxiCenter has function called assignTrip(int tripID), which iterates on the unassignedTrips vector, assigns it to the driver and removes the assigned trip from the unassignedTrips vector. now, to make the calculation on other thread, I created the class shown above, and have a ThreadPool in the taxiCenter. when the addTrip(TripInfo* tripInfo) method is called, I will create new PathCalculation object, new CalculatePathTask, add the PathCalculation to a vector in taxiCenter, and handle the task to the threadPool. Then, when I actually need to assign the trip, I will iterate on the tripInfo's. when it will find the driver to assign the trip to, it will construct the Trip object from the tripInfo and the vector of path from the PathCalculation. all I want to do is pass the path vector to the Trip object without copying too much, because the path vector could be big (2k points), and copying this vector around looks too resource-consuming to me, so I want to pass reference to that vector. of course, if I will copy the vector and then return it, there would be no problem in the means of the implementation, but It might consume too much resources(repeated copy of vector with 2k points is not efficient).

How you would suggest to solve this problem? I am ready to everything, change the way the taxiCenter work, redesign the multithreaded calculation, everything. I just want a well designed solution that will be considered as good-code in practice.

Thanks again, David

Unlocking the mutex allows the contents of `path` to change before that value is returned. But you *have* to unlock the mutex,so conceptually return a copy of the `path` vector and quite likely the `Point` objects held in the vector. You haven't provided enough details of other methods that can access the `_path` vector, nor details on your `Point` object, but to be completely MT-safe the specifics of what those methods access and modify and the details of your `Point` implementation are extremely important. Full MT-safety with C++ objects can get nasty, and exact details *really* matter. — Andrew Henle, Jan 16 '17 at 23:23
I return const reference, I just want to avoid copying the path vector too much times, because this vector will be copied later into a object. I do not use the reference outside, the use will be vector path = pathcalc.getPath();. given this information, and the edit of the post above, do you think unlocking and then returning the vector will be thread-safe? — David, Jan 17 '17 at 00:31
Just return a copy, by value. `vector path = pathcalc.getPath();` will then use a move constructor, which is very cheap. There won't be another full copy of array elements. — Igor Tandetnik, Jan 17 '17 at 02:25
@David *I return const reference* The problem isn't the reference that's returned by `getPath()`. The problem is the vector object the reference is to can be modified by other threads after you return a reference to it - even if that reference is `const`. From your description, the suggestion to return a copy of `_path` should work: `... std::vector copy = _path; pthread_mutex_unlock(&pmtx); return(copy);` Note that the copy is made *while the mutex is locked*. Then you can unlock the mutex and return the copy by value. — Andrew Henle, Jan 17 '17 at 10:59

unlocking mutex after returning object vector variable

0 Answers0