heap corruption when sharing vector between two threads

Question

In below code, the sv object in main function is created on stack but doesn't go out of scope. But I am getting occasional heap corruptions. When run under valgrind I see illegal read/write warnings

However if I create sv object on heap, I didn't see any issues.

CODE BELOW

#include <iostream>
#include <thread>
#include <functional>
#include <mutex>

mutex m;

struct SharedVec
{
public:
std::vector<int> v;
};

void check(SharedVec * sv)
{
    while (true) {
        m.lock();
        sv->v.push_back(2);
        m.unlock();
    }
}

int main()
{
    SharedVec sv; // this shared object which has vector member causing heap corruption
//although in scop
//SharedVec * sv = new SharedVec(); // if i create on heap, no issues seen

    std::thread t(check, std::bind(&sv));
    while (true)
    {
        std::vector<int> tmp;
        m.lock();
        tmp = sv.v;
        sv.v.clear();
        m.unlock(); 
    }

    t.join();  // Wait for the thread to finish
    return 0;
}

Answer 1

Not really an answer since your code did not compile. But here is some feedback on your code to help you with multithreading/locking of data in a safer way.

#include <iostream>
#include <future>
#include <functional>
#include <mutex>
#include <vector>

// don't use globals, specialy not a mutex
// so put mutex in SharedVec
// and give SharedVec some threadsafe operations

struct SharedVec
{
public:
    void push_back(int value)
    {
        // never use naked lock/unlock on mutex it is not exception safe
        std::scoped_lock<std::mutex> lock{ m }; 
        v.push_back(value);
    }

    void clear()
    {
        std::scoped_lock<std::mutex> lock{ m }; 
        v.clear();
    }

    auto get_copy()
    {
        std::scoped_lock<std::mutex> lock{ m };
        return std::vector<int>{v};
    }

private:
    // keep the lock with the object that you want to be threadsafe
    std::mutex m;
    std::vector<int> v;
};

int main()
{
    // when sharing data between threads (that do not synchronize
    // with each other use a shared pointer to the shared data)
    auto sv = std::make_shared<SharedVec>();

    // async has better abstraction then thread
    // capture shared pointer by value, extending its lifetime
    auto future = std::async(std::launch::async, [sv]
    {
        while (true)
        {
            sv->push_back(2);
        }
    });

    while (true)
    {
        // by making a get_copy function it is more clear what your code is doing
        std::vector<int> tmp = sv->get_copy();
        sv->clear();
    }

    future.get(); // synchronize
    return 0;
}