CODEWITHSUNDEEP
multithreadingc++11stdthread
Sven Hager
Sven Hager

Reputation: 3194

C++ 11 std::thread strange behavior

I am experimenting a bit with std::thread and C++11, and I am encountering strange behaviour. Please have a look at the following code:

#include <cstdlib>
#include <thread>
#include <vector>
#include <iostream>

void thread_sum_up(const size_t n, size_t& count) {
  size_t i;
  for (i = 0; i < n; ++i);
  count = i;
}

class A {
public:
  A(const size_t x) : x_(x) {}

  size_t sum_up(const size_t num_threads) const {
    size_t i;
    std::vector<std::thread> threads;
    std::vector<size_t> data_vector;
    for (i = 0; i < num_threads; ++i) {
      data_vector.push_back(0);
      threads.push_back(std::thread(thread_sum_up, x_, std::ref(data_vector[i])));
    }

    std::cout << "Threads started ...\n"; 

    for (i = 0; i < num_threads; ++i)
      threads[i].join();

    size_t sum = 0;
    for (i = 0; i < num_threads; ++i)
      sum += data_vector[i];
    return sum;
  }

private:
  const size_t x_;
};

int main(int argc, char* argv[]) {
  const size_t x = atoi(argv[1]);
  const size_t num_threads = atoi(argv[2]);
  A a(x);
  std::cout << a.sum_up(num_threads) << std::endl;
  return 0;
}

The main idea here is that I want to specify a number of threads which do independent computations (in this case, simple increments). After all threads are finished, the results should be merged in order to obtain an overall result.

Just to clarify: This is only for testing purposes, in order to get me understand how C++11 threads work.

However, when compiling this code using the command

g++ -o threads threads.cpp -pthread -O0 -std=c++0x

on a Ubuntu box, I get very strange behaviour, when I execute the resulting binary. For example:

$ ./threads 1000 4
Threads started ...
Segmentation fault (core dumped)

(should yield the output: 4000)

$ ./threads 100000 4
Threads started ...
200000

(should yield the output: 400000)

Does anybody has an idea what is going on here?

Thank you in advance!

Upvotes: 2

Views: 722

Answers (3)

rabensky
rabensky

Reputation: 2934

Your code has many problems (see even thread_sum_up for about 2-3 bugs) but the main bug I found by glancing your code is here:

data_vector.push_back(0);
threads.push_back(std::thread(thread_sum_up, x_, std::ref(data_vector[i])));

See, when you push_back into a vector (I'm talking about data_vector), it can move all previous data around in memory. But then you take the address of (reference to) a cell for your thread, and then push back again (making the previous reference invalid)

This will cause you to crash.

For an easy fix - add data_vector.reserve(num_threads); just after creating it.

Edit at your request - some bugs in thread_sum_up

void thread_sum_up(const size_t n, size_t& count) {
  size_t i;
  for (i = 0; i < n; ++i); // see that last ';' there? means this loop is empty. it shouldn't be there
  count = i; // You're just setting count to be i. why do that in a loop? Did you mean +=?
}

Upvotes: 2

Dave S
Dave S

Reputation: 21058

As you resize the vector with the calls to push_back, it is likely to have to reallocate the storage space, causing the references to the contained values to be invalidated. This causes the thread to write to non-allocated memory, which is undefined behavior.

Your options are to pre-allocate the size you need (vector::reserve is one option), or choose a different container.

Upvotes: 1

Nevin
Nevin

Reputation: 4853

The cause of your crash might be that std::ref(data_vector[i]) being invalidated by the next push_back in data_vector. Since you know the number of threads, do a data_vector.reserve(num_threads) before you start spawning off the threads to keep the references from being invalidated.

Upvotes: 1

Related Questions