Seg Fault resulting from push_back call on vector (threads linux)

Question

So what I'm trying to do is write a program that creates a series of child threads that take the arguments using the pthread_create method and uses the parameter passed in to do more manipulation and so on. The parameter I'm trying to pass in is a vector argument called reduce_args_. this is the header information for the struct ReduceVector.

typedef vector<string> StringVector;

// a data structure to maintain info for the reduce task
struct ReduceArg
{
  ReduceArg (void);  // constructor
  ~ReduceArg (void); // destructor

  pthread_t tid;  // thread id of the reduce thread
  StringVector files_to_reduce; // set of files for reduce task
};

// more typedefs
typedef vector<ReduceArg *> ReduceVector;

now the issues comes when I call push_back here:

for(int i = 0; i < num_reduce_threads_ ; i++){
            reduce_args_.push_back(phold);
        int count = 0;
        for(ShuffleSet::iterator it = shuffle_set_.begin(); it!=shuffle_set_.end(); ++it){
            string line = *it;
            string space = " ";
            string file = line.substr(0, line.find(space)) + ".txt";

            if (count < num_reduce_threads_){
                cout << reduce_args_[i+1];
                (reduce_args_[i+1] -> files_to_reduce)[count] = file;
                            //(reduce_args_[i+1] -> files_to_reduce).push_back(file);
             }
             count++;
            //cout << ((reduce_args_.back())->files_to_reduce).back()<< endl;
    }
}

both of those push_back methods cause a seg fault. the shuffle set is just a set and is outputting strings. and as noted in the .h file, the files_to_reduce is a string vector. So what I'm trying to do is access the files_to_reduce and push_back a string onto it, but each time I get a seg fault. The reduce_args_ obj is declared as below:

ReduceArg* plhold;
    reduce_args_.push_back(plhold);
    ((reduce_args_.back()) -> files_to_reduce).push_back("hello");
    for (int i = 0; i < this->num_reduce_threads_; ++i) {
      // create a placeholder reduce argument and store it in our vector
      (reduce_args_.push_back(plhold));
    }

thanks for the help!!

Not a direct solution but could save you a lot of trouble: If you're using C++11, please prefer `std::thread`. — Aleph, Oct 21 '13 at 18:50
Looks like you're pushing an uninitialized pointer into the vector `reduce_args_` then trying to access it. — Mgetz, Oct 21 '13 at 18:53
if the application crashes in push_back it means that the actual problem has nothing to do with the vector, but with a heap corruption that you cause earlier by writing over another arrays' bounds, in that case we'd need more code to be able to point you in the right direction. It might also be what Mgetz is saying — Radu Chivu, Oct 21 '13 at 18:55
Lots of spurious parentheses by the way: Just write `reduce_args_.back()->files_to_reduce.push_back("hello");` — Roddy, Oct 21 '13 at 18:58
basically you guys were right, I forgot to initialize it and as a result was accessing undefined memory outside of the bounds. Then I needed to move that initializing statement inside the loop. Thanks! — Michael Nakayama, Oct 21 '13 at 22:04

Roddy · Accepted Answer · 2013-10-21T19:06:07.380

4

This:

ReduceArg* plhold;
reduce_args_.push_back(plhold);

Unless you've hidden some important code, you're pushing an uninitialised pointer, so the next line will cause chaos.

Possibly you meant this?

ReduceArg* plhold(new ReduceArg);

..but I suspect you haven't properly thought about the object lifetimes and ownership of the object whose address you are storing in the vector.

In general, avoid pointers unless you know exactly what you're doing, and why. The code as posted doesn't need them, and I would recommend you just use something like this:

typedef vector<ReduceArg> ReduceVector;

....
reduce_args_.push_back(ReduceArg());
reduce_args_.back().files_to_reduce.push_back("hello");
for (int i = 0; i < num_reduce_threads_; ++i) {
  // create a placeholder reduce argument and store it in our vector
  (reduce_args_.push_back(ReduceArg());
}

edited Oct 21 '13 at 19:06

answered Oct 21 '13 at 18:55

Roddy

66,617
42
165
277

@MGetz Yes, but: It's difficult to know where to start given the posted code. My guess is that he shouldn't be storing pointers in the first place. – Roddy Oct 21 '13 at 19:02
I would suggest using [unique_ptr](http://en.cppreference.com/w/cpp/memory/unique_ptr), by declaring `typedef vector> ReduceVector;` if pointers are required. – Steve Oct 21 '13 at 19:06
That was it! I didn't realize I needed to initialize the plhold with new ReduceArg! Thank you so much! – Michael Nakayama Oct 21 '13 at 22:03

Seg Fault resulting from push_back call on vector (threads linux)

1 Answers1