CODEWITHSUNDEEP
c++multithreadingvector
mαττjαĸøb
mαττjαĸøb

Reputation: 335

C++ multiple threads and vectors

I tried various implementations of the following algorithm but always ended up with a crash after a while the program runs...

I have a base object

class Element
    {
    public:
        int a;
        float p;
        Element(int _a, float _p=1.0): a(_a), p(_p){};
    };

of which I create a vector and include in a Buffer object.

class Buffer
    {
    public:
        Buffer(){};

        vector<Element> raw;       
        vector<Element> optimised; // DATA CALCULATED BASED ON RAW

        void addElement(int _a,float _p=1.0) // FILL THE RAW BUFFER
        {
            raw.push_back(Element(_a,_p));
        }

        void compute()  // COMPUTE THE OPTIMISED BUFFER
        {
            float t;
            int i;
            for(std::vector<Element>::iterator it = raw.begin(); it != raw.end(); ++it) 
            {
                optimised.push_back(Element(it->a,it->p));
                // DO SOME COMPUTATIONALLY INTENSIVE CALCULATIONS
                for(i=1; i<9999999; i++)
                    t = 9./i;
            }
        };

        void clear() // ERASE BOTH BUFFERS
        {
            raw.clear();
            optimised.clear();
        }
    };

I have a declaration of a single Buffer object - responsible for capturing the current data stream - and a vector of Buffer objects - behaving like a history/queue of the previously created buffers.

Buffer buffer;
vector<Buffer> queue;

The main thread is responsible of filling the buffer object and - once a series is complete - submit the buffer into the queue. As soon as a new buffer is added to the queue a Compute() function is called on a separate thread to analyse the recently submitted data.

//ADD THE CURRENT BUFFER TO THE QUEUE
queue.push_back(buffer);

//RUN 'COMPUTE' IN PARALLEL/BACKGROUND ON THE LAST SUBMITTED BUFFER
std::thread t(&Buffer::compute, &queue.back());
t.detach();

//CLEAR THE BUFFER, READY FOR A NEW SERIES
buffer.clear();

The program complies fine and launches but it crashes during execution (sometimes after just one buffer is submitted, sometimes after a few... it generally 'works for longer' if there is only one buffer at a time in the queue).

Do I need to use mutex locks in this situation ? If so, where ?

Do you have any suggestion on how to optimise the collection of the data (fill the 'buffer' object and submit it into the queue) - I think AddElement() is a bit unnecessarily expensive ?

ANY HELP APPRECIATED!

Thanks

Upvotes: 1

Views: 3278

Answers (1)

Sorin
Sorin

Reputation: 11968

The problem is with &queue[last]. That gives you a pointer to where the vector currently stores the buffer. If vector reallocates (push_back can do that), then the pointer is invalid.

There are a few solutions to this:

  • Store pointer in the queue vector. Something like vector<unique_ptr<Buffer>> queue will work (and makes sure you don't accidentally leak the memory).
  • Use a datastructure that will not invalidate invalidate the pointers when modified. list and deque will work.
  • Make sure the vector doesn't reallocate. You can do a resize(x) initially, and then keep track of the last yourself.

Update: Add a code sample. This compiles and runs fine on Coliru (http://coliru.stacked-crooked.com/)

#include <memory>
#include <vector>
#include <iostream>
class Buffer {};

int main()
{
    std::unique_ptr<Buffer> buffer {new Buffer()};
    std::vector<std::unique_ptr<Buffer>> queue;
    for (int i = 0; i < 10; i++) {
        buffer.reset(new Buffer());
        // Do things to buffer;
        queue.push_back(move(buffer));
    }
    std::cout << queue.size() << std::endl;
}

Upvotes: 2

Related Questions