dereferencing this causes Segmentation fault

Question

I have the following functions

LinearScheme::LinearScheme() {
    cout << " empty constructor" << endl;
}


void LinearScheme::init(
    int tableId,
    std::string &basePath,
    std::vector<size_t> &colElemSizes,
    TupleDescMap &tupleDescMap,
    size_t defaultMaxFragmentSize,
    int numCols,
    BoundBases &bounds,
    std::vector<int> &colsPartitioned ) 
{
    // This linear scheme ignores bounds
    // it could be improved to use colsPartitioned for ordering (TODO)
    cout << "init Linear Scheme " << endl;

    *this = LinearScheme(); //SEGFAULTS HERE

    cout << "after cons here?" << endl;
    // init private fields    
    this->tableId_ = tableId;
    this->basePath_ = basePath;
    this->colElemSizes_ = colElemSizes;
    this->numCols_ = numCols;
    this->tupleDescMap_ = tupleDescMap;
    this->numFragments_ = 0;
    this->defaultMaxFragmentSize_ = defaultMaxFragmentSize;

    // fragmentSizesFilename_ init    
    fragmentSizesFilename_  = basePath_ + boost::lexical_cast <string>(tableId_)
        + "_cs";
    struct stat st;
    // open existing file if exists. Create new otherwise. 
    if (stat(fragmentSizesFilename_.c_str(), &st) == 0) // file existed
        openExisting();
    else
        createNew();
}

The reason I am initializing in init rather than constructor is because LinearScheme extends a PartitionScheme (super class with virtual methods) class and another class does that where the constructor is used recursively.

I have a QuadTree class which does the same initialization because each QuadTree constructor is applied recursively. *this = QuadTree(bounds, maxSize) line in the init function of QuadTree class works just fine.

however, this line in the other subclass (LinearScheme) *this = LinearScheme() cause a Seg fault.

Any ideas why this might happen?

EDIT Also replacing the line:

*this = LinearScheme()

with this:

*this;

or removing it overall gets rid of the Seg Fault ... why?

Answer 1

Sounds like incorrect factory method / builder / deferred construction usage. For many of these object creation patterns function that constructs your objects should be a static method because there doesn't yet exist an instance to manipulate. In others you potentially manipulate an already constructed instance. In either case if you are actually constructing the object of the class type within the function you should be using new and eventually returning it.

If you are instead going for a helper method to assist with initialization then you simply shouldn't be constructing the object within the method itself, and you should just be initializing parts of it within your helper.

A factory pattern example:

LinearScheme* LinearScheme::create(...all_your_args....) {

    /* construct the thing we are building only if it 
    *  pass any arguments into him that he can handle directly if you'd like
    */
    LinearScheme *out = new LinearScheme(...);

    /* do whatever else you have to do */
    ....
    return out;
}

or this helper of sorts that you seem to want

/* this time let's just do 'init' on your object */
void LinearScheme::init(....args....) {
    /* possibly check if init has been done already */
    if ( this->init ) return;

    /* proceed to do your initialization stuff
     * but don't construct the 'this' instance since it should already exist
     */

    this->init = true; //so we don't init again if you don't need multiple init's
}

Alternatively you can consider the delegate constructor methods in C++11 alex mentions.

However neither of these really strikes me as being the actual problem here.

It's not working because either you probably don't even have a valid *this to deference. This could be because of your usage, or it could be because one failed to create potentially because of infinite recursion.

Here's a wikipedia link on the pattern: http://en.wikipedia.org/wiki/Factory_method_pattern

Given what you have said about having to keep passing a dozen arguments around both to parent classes and for your recursive construction, one suggestion you could consider is making a small config struct that you pass along by reference instead of all the discrete parameters. That way you don't have to keep adjusting every signature along the way each time you add / remove another parameter.

The other idea is to seperate entirely the construction of one of your objects from the responsibility of knowing how, where, and when they should be contructed and inserted into your hierarchy. Hard to say without understanding how you will actually be using LinearSchme and what the interface is.

Answer 2

"...in the other subclass (LinearScheme) *this = LinearScheme()"

"The LinearScheme constructor is empty: LinearScheme::LinearScheme()"

if *this is a subclass of LinearMethod, LinearMethod's constructor should already have been called and this line is useless. Besides it calls assignment operator - is it properly defined?

It is better to rely on built-in mechanism of constructing of objects. If you want to avoid code repetition, use C++11 delegating constructors feature. It was specially designed to eliminate "init" methods.

Although, "If there is an infinitely recursive cycle (e.g., constructor C1 delegates to another constructor C2, and C2 also delegates to C1), the behavior is undefined."

So it is up to you to avoid infinite recursion. In your QuadTree you can consider creating nullptr pointers to QuadTreeNode in constructor.