Vector push_back of one object results in a vector of enormous size

Question

i'm working on a project in c++, and I have a vector of objects, where I want to push_back an object on the existing vector. However, when checking the size before and after the object is added, the size goes from 0 to 12297829382473034412 which puzzles me greatly. The code in question is the addCommodity function below. (I have created a smaller example of the same problem further down, so skip to "SMALL PROBLEM")

void Instance::addCommodity(std::vector<std::string> & tokens) {
  /*if(tokens.size()!=5){
    std::cerr << "Error in commodity data format"<< std::endl;
    exit(-1);
  }*/
  // size_t so = std::atoi(tokens[1].c_str());
  // size_t si = std::atoi(tokens[2].c_str());
  // size_t demand = std::atoi(tokens[3].c_str());
  // size_t ti = std::atoi(tokens[4].c_str());
  std::cout << "size: " << this->_commodities->size() << "\n"; 

  this->_commodities->push_back(Commodity(1,2,3,4));  // ???

  std::cout << "size: " << this->_commodities->size() << "\n";
}

Here I have commented out the parts of the code which are used to read data from a string which was loaded from a file. Commodity is defined as follows:

#include "commodity.h"

Commodity::Commodity(size_t so, size_t si, size_t d, size_t ti):
  _source(so),
  _sink(si),
  _demand(d),
  _maxTime(ti)
{}

Commodity::~Commodity(){}

size_t Commodity::getSource() const{
  return _source;
}

size_t Commodity::getSink() const {
  return _sink;
}

size_t Commodity::getDemand() const {
  return _demand;
}

size_t Commodity::getTime() const {
  return _maxTime;
}

Where Instance is initialised as:

Instance::Instance(std::shared_ptr<Param> p, size_t n):
  _params(p),
  _nNodes(n)
{
  this->_commodities.reset(new std::vector<Commodity>());
  this->_arcs.reset(new std::vector<Arc>());
  }

As mentioned before my issue lies in the addCommodity code, when trying to push_back a Commodity. Hopefully this is enough code to identify any stupid mistakes that I have made. I left out most of the other code for this project as it doesn't seem to have an impact on the addCommodity function.

The output received when calling the function is:

size: 0
size: 12297829382473034412

SMALL PROBLEM

Instead of showing all the code, I have run the push_back on the vector in main:

#include <iostream>
#include <memory>
#include <sys/time.h>
#include <vector>


#include "commodity.h"

int main(int argc, char* argv[]){


 std::shared_ptr< std::vector<Commodity>> commodities;

  commodities.reset(new std::vector<Commodity>());

  std::cout << "size: " << commodities->size() << "\n";

  size_t a = 1;
  size_t b = 2;
  size_t c = 3;
  size_t d = 4;

  commodities->emplace_back(Commodity(a,b,c,d));

  std::cout << "size: " << commodities->size() << std::endl;

  return 0;

}

This is basically a smaller instance of the same code. The commodity cpp and h files are as follows:

#include "commodity.h"

Commodity::Commodity(size_t so, size_t si, size_t d, size_t ti):
  _source(so),
  _sink(si),
  _demand(d),
  _maxTime(ti)
{}

Commodity::~Commodity(){}

size_t Commodity::getSource() const{
  return _source;
}

size_t Commodity::getSink() const {
  return _sink;
}

size_t Commodity::getDemand() const {
  return _demand;
}

size_t Commodity::getTime() const {
  return _maxTime;
}

The header file:

#ifndef CG_MCF_COMMODITY_H
#define CG_MCF_COMMODITY_H

#include <stdlib.h>

class Commodity {

 public:

  Commodity(size_t so, size_t si, size_t d, size_t t);

  ~Commodity();

  size_t getSource() const;

  size_t getSink() const;

  size_t getDemand() const;

  size_t getTime() const;

 private:

  size_t _source;

  size_t _sink;

  size_t _demand;

  size_t _maxTime;

};

#endif /*CG_MCF_COMMODITY_H*/

The output received when calling the function is:

size: 0
size: 12297829382473034412

Can you create a **complete** code example that anyone can copy-and-paste, run, and compile themselves? — Code-Apprentice, Jan 18 '17 at 16:00
Can you show us the of the class "Instance"? It is most likely that there is somewhere a pointer issue. — Gene, Jan 18 '17 at 16:00
@EliasKosunen: What? That's not a rule. You would heap allocate a standard library container for the same reasons you would heap allocate any other object. — Benjamin Lindley, Jan 18 '17 at 16:06
@BenjaminLindley A `std::vector` is basically composed of three data members: size, capacity (both size_t) and a pointer to the data which is dynamically allocated. There's no benefit of putting the whole vector on the heap when the data is already there. — Elias Kosunen, Jan 18 '17 at 16:09
@EliasKosunen: What if you need a persistent address for the vector? For example, if you want shared ownership. — Benjamin Lindley, Jan 18 '17 at 16:10
@EliasKosunen There is if you need ownership semantics. it all depends on the use case. — NathanOliver, Jan 18 '17 at 16:11
When you use `push_back` creating a `Commodity` object inside of the vector may invoke the copy constructor (not shown here) which may have some side effects. You may start with replacing `push_back` with `emplace_back` (uses perfect forwarding). — jszpilewski, Jan 18 '17 at 17:23
hex(12297829382473034412) = 0xaaaaaaaaaaaaaaac.
This looks a lot like a malloc sentinel value for something like uninitialized memory or freed memory -- the _commities vector is being corrupted somehow. — joeking, Jan 18 '17 at 20:55
I [cannot reproduce the issue in your minimal example](http://melpon.org/wandbox/permlink/m9UaBbQ9wFVFwiuJ). — Vittorio Romeo, Jan 20 '17 at 11:40
Did you 100% confirm that your minimal example replicates the problem? (Because if the real `Commodity` class violates the rule of 0/3/5 more seriously than the example class does, that would explain everything.) — David Schwartz, Jan 20 '17 at 12:01
The advice shouldn't be "never allocate a container on the heap" but "don't allocate anything on the heap that you don't need to." On the one hand, the question uses a smart pointer, so that's an improvement over what you normally see. On the other hand, we're talking about private member variables, so there shouldn't be any shared ownership. — Max Lybbert, Jan 20 '17 at 12:25
You're focused on the wrong thing. I have no problem using`push_back` or`emplace_back` to turn an empty `std::vector` into a one-element `std::vector`. It works even if I wrap the vector in a `std::shared_ptr`. You appear to have undefined behavior somewhere else. Note: undefined behavior can cause unrelated valid code to misbehave ( http://blog.llvm.org/2011/05/what-every-c-programmer-should-know.html ). There's no defined scope that might be affected by undefined behavior. — Max Lybbert, Jan 20 '17 at 12:36

score 1 · Answer 1 · answered Jan 20 '17 at 12:05

Your Commodity class violates the rule of 0/3/5.

Your code (inexplicably) does this:

commodities->emplace_back(Commodity(a,b,c,d));

This is really strange. Presumably, you're calling emplace_back to avoid having to construct a separate Commodity from the one in the vector. But you force that to happen by explicitly constructing a separate Commodity as the parameter to emplace_back.

That invokes Commodity's copy constructor to construct the Commodity in the vector as a copy of the one you explicitly created. Except Commodity doesn't have one. Most likely, the real Commmodity class needs one, since it has a destructor.

Vector push_back of one object results in a vector of enormous size

1 Answers1