1

I need to pass into a thread by value a block of floats that are a pointer returned by a third party library. Something like this:

void TestThread(std::vector<float> data)
{
  std::cout << data.size();
}

void Thready ()
{
  vector<thread> threads(10);  // this actually varies at runtime.

  for (int i = 0; i < 10; i++)
  {
      float* data = HeapsOfFloats();
      int numberOfFloats = NumberOfFloats();
      std::vector<float> dataVector(data, data + numberOfFloats);
      threads[i] = std::thread::thread([&]{TestThread(dataVector)});
  }

  for (int t = 0; t < 10; t++) threads[t].join();
}

After 1-n iterations of the loop, my program is crashing.

empty
  • 5,194
  • 3
  • 32
  • 58
  • or it crashes because you're spamming threads – deW1 Jan 13 '17 at 23:16
  • _because floats are not integral types so there's still pointers hiding in this._ How so? – Algirdas Preidžius Jan 13 '17 at 23:19
  • Besides Oleg's very true comment about passing `dataVector` by reference to your lambda (consider using `[vec = std::move(dataVector)] { TestThread(vec) }` to cleanly move data to the closure), everything looks fine from here and I suspect that your crash is somewhere in `HeapsOfFloats` or `NumberOfFloats`. I don't understand your comment about floats not being integral types and pointers. – zneak Jan 13 '17 at 23:48
  • 1
    Integral means only that type is integer. Nothing more or less. Floating point numbers are also store as normal memory blocks. – Logman Jan 14 '17 at 00:05
  • Removed integral references in OP and in comments. – empty Jan 14 '17 at 00:28

1 Answers1

4

Edit: You are capturing vector by reference (to object) that is going to be destroyed after your loop iteration ends, if you are lucky and function call inside lambda has already copied it - it works, if you are unlucky - there's nothing to copy and you get a crash. Given that you do want to use a copy anyway I suggest capturing it by copy into lambda and then passing it by ref:

void TestThread(const std::vector<float>& data)
{
  std::cout << data.size();
}

void Thready ()
{
  vector<thread> threads(10);  // this actually varies at runtime.

  for (int i = 0; i < 10; i++)
  {
      float* data = HeapsOfFloats();
      int numberOfFloats = NumberOfFloats();
      std::vector<float> dataVector(data, data + numberOfFloats);
      threads[i] = std::thread::thread([dataVector]{TestThread(dataVector)});
  }

  for (int t = 0; t < 10; t++) threads[t].join();
}

Bonus: your code (and mine) incurs two copies of float data but you could get away with at least one less copy if you can write in c++14:

...
 threads[i] = std::thread::thread([dataVector(std::move(dataVector))]{TestThread(dataVector)});
...

For pre C++14 solutions look here or wrap your vector in smth like shared_ptr that would be cheaper to copy


I don't think there's anything wrong with the way you grab your floats (if only you spawn way too many threads).

However I think your problem is elsewhere: you are destructing threads at each iteration, but they have to be manually detached or joined to avoid termination, sometimes you are lucky and they run to termination before your loop cycle ends, sometimes - you are not. Just consider detach at the end of iteration if you don't care about them and dont plan to use thread handles later

Community
  • 1
  • 1
Oleg Bogdanov
  • 1,712
  • 13
  • 19