0

I have a library that is all tested thoroughly through google test suite. I am trying to keep it "pimpl" clean, but I'm running into a segfault I can't quite figure out.

Relevant Code:

Interface.h:

class Interface{
public: 
Interface();
void Function(const int argument);

private:
std::unique_ptr<Implementation> Implement;    
std::unique_ptr<DependencyInjection> Injection1, Injection2;    
};

Interface.cpp:

Interface::Interface()
: Injection1(new DependencyInjection()),
Injection2(new DependencyInjection()),
Implement(new Implementation(*Injection1, *Injection2)) {}

void Interface::Function(const int argument){ Implement->Function(argument); }

Implementation.h:

class Implementation{
public: 
Implementation(AbstractInjection &injection1, AbstractInjection &injection2);
void Function(const int argument);
private:
AbstractInjection Injection1, Injection2;
};

Implementation.cpp

Implementation::Implementation(AbstractInjection &injection1, AbstractInjection &injection2)
: Injection1(injection1),
Injection2(injection2) {}

void Implementation::Function(const int argument){
injection1.Function(argument); } // code from here out is all well tested and works 

So when I create the interface and call Interface.Function() the code segfaults when it tries to evaluate Implementation.Function(). I've ran gdb through everything I can think of, all the pointers are non-null.

If I just create a test that looks like

std::unique_ptr<DependencyInjection1> injection1(new DependencyInjection());
std::unique_ptr<DependencyInjection2> injection2(new DependencyInjection());
std::unique_ptr<Implementation> implement(new Implementation(*injection1, *injection2));
implement->Function(0);

The code works fine and does not segfault

But if I create a test like

Interface iface;
iface.Function(0);

it will segfault.

I am new to the whole unique_ptr thing, but I have a suspicion that isn't the larger problem. It may be a red herring, I don't know.

shavera
  • 803
  • 1
  • 8
  • 18

2 Answers2

1

The problem should actually pop as as a warning.

Initializers are done in the order in which they appear in the class definition, not in which they appear in the constructor!

Switch it to:

class Interface{
public: 
Interface();
void Function(const int argument);

private:
std::unique_ptr<DependencyInjection> Injection1, Injection2;
std::unique_ptr<Implementation> Implement;

};

From here: C++: Initialization Order of Class Data Members, this is "12.6.2 of the C++ Standard"

Community
  • 1
  • 1
IdeaHat
  • 7,641
  • 1
  • 22
  • 53
  • -Edit: Nope, I had them in the wrong order after all. Sorry, That was right! Thanks a bunch! Tests Pass now. (previous comment): Sorry, I had them in the right order in the code, But I didn't write them in the right order in my post. – shavera Apr 16 '14 at 14:43
  • @Alex I'd like to say that your interface probably shouldn't own Injection1 and Injection2, your making two copies of the Injection classes. Also you'll get slicing when assigning to your instances of "AbstractInjection" in you implementation call. – IdeaHat Apr 16 '14 at 14:47
  • yeah that's more an artifact of the abstraction when I went to copy it to here. It's really more like Class A; Class B: Class A; and Classes X, Y: Class B, Class Z:A . Then the implementation has a constructor(B, B, A) and the interface injects (X, Y, Z). In the case of testing it's (MockX, MockY, MockZ) – shavera Apr 16 '14 at 19:34
1

You've got a wrong order of member fields, they are initialized in order they are declared in the class. So implement is initialized before both injections. Use -Werror=reorder to get compiler error (for GCC and probably CLang)

user3159253
  • 16,836
  • 3
  • 30
  • 56