5

I have a abstact base class that calls a virtual method in it's constructor. after passing a shared_ptr of the base class the implementation of the method is not found. 

class a 
{
 public:
   a() { fill(); }
 protected:
   virtual void fill() = 0;
}

class b : public a
{
public:
   b() : a();
protected:
   virtual void fill() { // do something }
} 
....

shared_ptr<a> sptr = shared_ptr<a> ( new b()): // error happens here on runtime

When executing this I get a SIGABRT because it tries to execute the virtual void fill() = 0;

Steve
  • 1,072
  • 11
  • 28
  • 2
    You should avoid instantiating shared pointers with the new key word entirely and prefer to use make_shared. EDIT: Hmmm, on second thoughts, this might not be true in polymorphic cases. – Ben J Nov 26 '13 at 14:08
  • @BenJ Re 2nd thoughts: Why not? At most, you'll get one atomic-aware upcast. That overhead is still probably negligible compared to the dynamic allocation itself. – Angew is no longer proud of SO Nov 26 '13 at 14:15
  • @Angew; on third thoughts, you're right; make_shared is definitely the way to go – Ben J Nov 26 '13 at 14:25

3 Answers3

7

You cannot call a pure virtual function from the constructor. At the time the constructor is running, the object is considered to be of the type being constructed, not of any derived type. Which means virtual dispatch "stops" at the type being constructed.

This means that calling fill() from the constructor of a will try to call a::fill(), regardless of any derived classes of which this a subobject can be part. And this of course fails miserably, since the function has no implementation.

Additionally, as @KerrekSB points out, your class needs a virtual destructor. Otherwise, you will get undefined behaviour if you ever delete a b instance via a pointer to a (which is quite likely when shared_ptr<a> is involved).

UPDATE Apparently, shared_ptr is capable of using the default deleter properties to work around the necessity for a virtual destructor, so your class is technically OK not having one. Still, without a virtual destructor, you class depends on being managed in std::shared_ptrs only; if you ever change that bit of design, you will run into trouble (and it will not be immediately obvious). I therefore suggest having a virtual destructor anyway.

Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
  • 2
    In this case, the shared pointer *won't* try to delete via a pointer to `a`. If it's initialised with `b*`, then it will use `std::default_deleter` and delete the correct type, whether or not there's a virtual destructor. – Mike Seymour Nov 26 '13 at 14:12
  • Dynamic dispatch is *not* suppressed during construction, it just has different behavior than what is expected before learning the details (aggravated by the fact that Java does the *wrong*/opposite thing) – David Rodríguez - dribeas Nov 26 '13 at 14:13
  • 2
    @MikeSeymour: That's very good to know. I didn't realize that's required. It's particularly noteworthy that the same does *not* work with `unique_ptr`, and you would have to supply your own deleter explicitly. – Kerrek SB Nov 26 '13 at 14:14
  • @DavidRodríguez-dribeas That's why I put the "partially" there. – Angew is no longer proud of SO Nov 26 '13 at 14:19
  • @Angew: That's why I did not downvote :) But I still find it misleading. The common description you can find, which I like better, is that the type of the object under construction *evolves* as the different constructors run. Dynamic dispatch always work, but the type to which the function is dispatched will differ depending on where the initialization is performed. – David Rodríguez - dribeas Nov 26 '13 at 14:21
  • @DavidRodríguez-dribeas Fair enough, I will elaborate on it a bit. – Angew is no longer proud of SO Nov 26 '13 at 14:24
1

"Never Call Virtual Functions during Construction or Destruction"

And you can use std::make_shared:

shared_ptr<a> sptr = std::make_shared<b>();
masoud
  • 55,379
  • 16
  • 141
  • 208
0

You shouldn't be calling virtual functions from constructors. Read this: https://stackoverflow.com/a/962148/1380817

Community
  • 1
  • 1
alex
  • 161
  • 1
  • 7