6
\$\begingroup\$

I have implemented a thread safe vector, based on std::thread and std::vector. I am new to many of the patterns used in threading and was wondering if there's a more efficient way the locking can be implemented? Also I'm not sure what the best name is to give variables that just get stored right away (i.e. in).

Here is SafeVector.hpp

#ifndef SAFEVECTOR_HPP
#define SAFEVECTOR_HPP

#include <vector>
#include <mutex>
#include <condition_variable>
#include <string>

class SafeVector {
public:
    SafeVector() : vec(), mut(), cond() {}
    SafeVector(const SafeVector& orig) : vec(orig.vec), mut(), cond() {}
    ~SafeVector() {}

    void insert(std::string in, const int index);
    void push_back(std::string in);
    std::string& operator[](const int index);
    std::vector<std::string>::iterator begin();
    std::vector<std::string>::iterator end();
    std::vector<std::string> toVector();

private:
    std::vector<std::string> vec;
    std::mutex mut;
    std::condition_variable cond;
};

#endif /* SAFEVECTOR_HPP */

Here is SafeVector.cpp

#include <string>
#include <utility>
#include "SafeVector.hpp"


void SafeVector::insert(std::string in, const int index)
{
    std::lock_guard<std::mutex> lock(mut);
    vec[index] = std::move(in);
    cond.notify_one();
}

void SafeVector::push_back(std::string in)
{
    std::lock_guard<std::mutex> lock(mut);
    vec.push_back(std::move(in));
    cond.notify_one();
}

std::string& SafeVector::operator[](const int index)
{
    return vec[index];
}

std::vector<std::string>::iterator SafeVector::begin()
{
    return vec.begin();
}

std::vector<std::string>::iterator SafeVector::end()
{
    return vec.end();
}

std::vector<std::string> SafeVector::toVector()
{
    return vec;
}

Currently, these are the only functions I need to use on vectors but if I decide to implement more, I'm assuming the rest would be done in the same way.

\$\endgroup\$
2
\$\begingroup\$

We probably want to be able to move-construct, so as well as this constructor:

SafeVector(const SafeVector& orig) : vec(orig.vec), mut(), cond() {}

We probably want also:

SafeVector(SafeVector&& orig)
  : vec{std::move(orig.vec)},
    mut{},
    cond{}
{}

I'd recommend being able to construct from a standard vector:

SafeVector(std::vector vec)
  : vec{std::move(vec)},
    mut{},
    cond{}
{}

Since we always default-initialize mut and cond, we could declare their initializers in the class definition, removing the need to have them in the constructors' initializer lists.

There's no need to specify an empty destructor - just let the compiler generate it.

I think we probably want one or more assignment operators.

Big question: how can we safely use iterators we get from begin() and end()? There's no way for client code to lock the mutex until it's finished using the iterators.

\$\endgroup\$
  • \$\begingroup\$ Re: Big Question. My use case is such that begin() and end() are only called when one thread is executing at a time. But good point. \$\endgroup\$ – northerner Mar 7 at 15:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.