1

I need to generate a bunch of random numbers and use(just read) them from different threads. Here's how I generate the numbers:

const std::vector<double>* Test::getRandomDoubles(int count)
{
    std::vector<double>* numbers = new std::vector<double>();
    numbers->reserve(count);

    for (int i = 0; i < itemsCount; i++)
    {
        srand((int)std::chrono::high_resolution_clock::now().time_since_epoch().count());
        numbers->emplace_back(rand() / 1000);
    }
    return numbers;
}

But I lose the numbers at some point:

std::future<void> SomeMethod(const std::vector<double>& numbers)
{
    return std::async( // breakpoint: numbers are available when creating the future.
        std::launch::async,
        [&]()
        {  // breakpoint: numbers gone. <-
            // Use numbers

And if helps, I call SomeMethod like:

// Just want to run some test for SomeMethod.
std::vector<void> futures;
for(int i = 0; i < 10; i++)
{
   std::vector<double> numbers = *getRandomDoubles(count);
   futures.emplace_back(SomeMethod(numbers));
}
for(int i = 0; i < 10; i++)
{
    futures[i].get();
}

Currently, I'm not calling delete on the numbers anywhere in my code.

What should I do to have numbers inside the async lambda?

Hossein Ebrahimi
  • 632
  • 10
  • 20
  • 1
    It's hard to see what is going wrong without seeing what is happening inside the lambda and what happens during `// Other stuff...`. Please try to create a [Minimal, Complete, Reproducible Example](https://stackoverflow.com/help/minimal-reproducible-example). – G. Sliepen Oct 02 '20 at 21:33
  • 1
    Never, ever use a `[&]` in an async or thread call. It isn't always wrong, but in my experience it will invariably become wrong at some point. **ANYTHING** that you put into a thread is something that you have to **THINK** about before using. It has to have enough lifetime to live throughout the thread. It has to be atomic or locked if used by multiple threads. So always, always name your captures by reference. Capture by assignment is less important since those will always be copies. Unless they are pointers, then this also applies to `[=]`. – Zan Lynx Oct 02 '20 at 22:44

2 Answers2

4

It is your responsibility to make sure that the lambda doesn't outlive variables it captures by reference.

If you return a lambda from a function, then by the time you call it, local variables it has captured are long dead. The same thing occurs if you run it asynchronously and return a future.

One way to overcome this problem is to capture by copy or by move.

Incidentally, getRandomDoubles() should not call new and return a pointer. C++ is not Java. Write

std::vector<double> Test::getRandomDoubles(int count) {
    std::vector<double> numbers;
    ...
    return numbers;
}
n. m. could be an AI
  • 112,515
  • 14
  • 128
  • 243
1

The vector you capture is a copy, not the one you dynamically allocated:

std::vector<double> numbers = *getRandomDoubles(count);

The moment this line is done, that original vector is gone forever as leaked memory. Make numbers a reference to avoid the copy. As a cautionary warning, also make sure you synchonize access to this shared mutable state if you're not going to copy or move it into the asynchronous code.

chris
  • 60,560
  • 13
  • 143
  • 205
  • I just made numbers a reference(vector&). Didn't help. – Hossein Ebrahimi Oct 02 '20 at 21:50
  • @HosseinEbrahimi, I don't know what to say then. While smelly, the question's code appears to me to be correct with this fix. This problem also reflects the described behaviour. I don't have a way of reproducing the problem. – chris Oct 02 '20 at 21:53