8

Is it legal C++ to create a worker-object on the stack in the destructor of some master-object and pass the this pointer of the master-object to the helper-object? The helper-object would then also call member functions of the master-object or access member-variables.

In other words, is the following legal C++?

struct MasterClass
{
  MasterClass (int data);

  ~MasterClass ();

  int data;
};

struct WorkerClass
{
  WorkerClass (MasterClass *m) : m (m) { }

  void do_some_work () { m->data = 42; }

  MasterClass *m;
};

MasterClass::MasterClass (int data)
: data (data)
{ }

MasterClass::~MasterClass ()
{
  WorkerClass w (this);

  w.do_some_work ();
}

int main ()
{
  MasterClass m (7);
}

I understand that the lifetime of the master-object ends once the destructor begins to execute. But I believe it is legal to call non-virtual member functions in the destructor of any object, which make use of the implicit this argument/parameter.

levzettelin
  • 2,600
  • 19
  • 32
  • 1
    It is legal, just like many other way to shoot yourself in the leg. Typically you should only perform resource cleanup in destructor, not some fancy finalization actions. – user7860670 Oct 21 '17 at 08:10
  • Yeah, thanks for the reminder @VTT. In the real counter-part to this I'm indeed just doing cleanup, but it's for a container that is a multi-dimensional tensor. Since cleanup for this type of object is quite complicated, I'm trying to reuse a component that I already have. – levzettelin Oct 21 '17 at 08:16
  • Is that a purely std question or a practical question? – curiousguy Oct 23 '17 at 02:44
  • @curiousguy As mentioned in my comment above, I am actually doing something like this. It lead to problems with the gcc address-sanitizer reporting use-after-scope errors during stack unwinding for an exception. Given then answer by "Passer By" and a lot of debugging, however, I now think this is a bug in the address-sanitizer. – levzettelin Oct 23 '17 at 07:06
  • Can you post real code that causes the error? – curiousguy Oct 23 '17 at 12:59
  • Nah, it's way too much code to post here; I didn't find the time to condense it. And as I said: I now believe the problem is with ASAN, so it doesn't really fit here. But the code is open-source, so I could point you to it. – levzettelin Oct 23 '17 at 19:52
  • Can the "work" fail? How is failure notified? – curiousguy Oct 26 '17 at 21:23
  • No, the work cannot fail. I'm just calling destructors. But the objects may be laid out in memory in a quite sophisticated pattern. You really are curious ;). – levzettelin Oct 27 '17 at 18:10

2 Answers2

7

Yes and no.

Yes, because its legal in this very short example you've shown.

No, because it might result in UB, there are some caveats surrounding usage of an object during destruction

TLDR It's always fine if you don't have any inheritance.

Now, for the cases where it is not fine to use an object during destruction.

The following cases will assume the following is already written

struct V;
struct A;
struct B;
struct D;

void foo(A* a = nullptr);

struct V {
    virtual void f();
    virtual void g();
};

struct A : virtual V {
    virtual void f();
};

struct B : virtual V {
    virtual void g();
    ~B() {
        foo();
    }
};

struct D : A, B {
    virtual void f();
    virtual void g();
    ~D() {
        foo(this);
    }
};

int main() {
    D d;
}

Calling virtual functions

Upon the destruction of x (aka as soon as its destructor is called)

If the virtual function call uses an explicit class member access and the object expression refers to the complete object of x or one of that object's base class subobjects but not x or one of its base class subobjects, the behavior is undefined.

Which means, if you use a explicit class member access to call a virtual function with a pointer pointing to the entirety of x, but somehow the pointer isn't the type of x nor its bases, the behaviour is undefined.

void foo(A* a) {
    static auto ptr = a;
    ptr->g();  // UB when called from ~B
               // ptr refers to B, but is neither B nor its base
}

Using typeid

If the operand of typeid refers to the object under construction or destruction and the static type of the operand is neither the constructor or destructor's class nor one of its bases, the behavior is undefined.

Likewise, if the operand refers to the object being destructed, yet somehow isn't the object and its bases, the behaviour is undefined.

void foo(A* a) {
    static auto ptr = a;
    typeid(*ptr);  // UB when called from ~B()
                   // ptr refers to B, but is neither B nor its base
}

Using dynamic_cast

If the operand of the dynamic_­cast refers to the object under construction or destruction and the static type of the operand is not a pointer to or object of the constructor or destructor's own class or one of its bases, the dynamic_­cast results in undefined behavior.

Same deal.

void foo(A* a) {
    static auto ptr = a;
    dynamic_cast<B*>(ptr); // UB when called from ~B()
                           // ptr refers to B, but is neither B nor its base
}

Conclusion

Now, if you think this is a fiasco and didn't understand what is going on, just don't pass this anywhere in a destructor.

All quotes from http://eel.is/c++draft/class.cdtor

Passer By
  • 19,325
  • 6
  • 49
  • 96
  • Did you mean to write `foo(this)` in the destructor of B? – levzettelin Oct 21 '17 at 09:18
  • 1
    @TobiasBrüll No, its not a typo. `foo` retains the pointer from the call from `~D` – Passer By Oct 21 '17 at 09:19
  • Hmm, isn't `a` then nullptr in all of your implementations of `foo`? In your first implementation of `foo` you then call `g()` on a nullptr, which is UB anyway. Or am I missing something? – levzettelin Oct 21 '17 at 09:21
  • 1
    `~D` gets called first, before the call to `~B`. Edited in a `main` to clarify – Passer By Oct 21 '17 at 09:21
  • Ahh, you assume that one would only ever create objects of type `D`. – levzettelin Oct 21 '17 at 09:22
  • Instead of that main function at the end, I would suggest either making A, B abstract or just putting a comment. In any case, calling foo() is UB because of the nullptr first and foremost, even before the fun with the partially destructed objects begins. – Javier Martín Oct 21 '17 at 09:32
  • @JavierMartín The example is saying how you _might_ get UB, instead of being unconditionally UB. Since `~D` gets called first, `ptr` shouldn't be a `nullptr`, no? Although, the more I look at it, the more I feel like the example isn't the best, its adapted from those in the standard but doesn't feel quite right. – Passer By Oct 21 '17 at 09:55
  • I know this is stupid, but how execution of `~D` first, make any difference to `foo()` call in `~B()`.I still see `a` as `nullptr` when getting executed from `~B()` – Gaurav Sehgal Oct 22 '17 at 08:29
  • 1
    @GauravSehgal But `ptr` is now `this` from `~D`. Dynamic initialization takes place only once – Passer By Oct 22 '17 at 09:52
  • @PasserBy Ahh..completely missed that.Thanks.One more thing can you please explain what do you mean when you say *ptr refers to B, but is neither B nor its base* How does it refer to `B`? – Gaurav Sehgal Oct 22 '17 at 10:15
  • @GauravSehgal It is pointing to `B` but is neither `B*` nor `V*`. I agree the wording sounds funky – Passer By Oct 22 '17 at 10:16
  • tl; dr: It's safe to keep around pointers and use them during c/dtion only when the dynamic type is increasing. During ction of a object with non virtual base class, the dynamic type can only increase. – curiousguy Oct 23 '17 at 02:40
1

Yes, this is legal, since the master object will not be destroyed before the termination of execution of the destructor.

However, this is not a good practice in general.

gsamaras
  • 71,951
  • 46
  • 188
  • 305
  • 1
    An object's lifetime is considered over at the start of the destructor, this might be benign in most cases but there are cases where it's not – Passer By Oct 21 '17 at 08:07
  • @PasserBy When is it not benign? – Gaurav Sehgal Oct 21 '17 at 08:08
  • @PasserBy yes. An object goes out scope first, then its destructor is called. However, its memory is not free'd upon completion of the destructor. It all depends on what you want to do with `this`, I will update. – gsamaras Oct 21 '17 at 08:10
  • @GauravSehgal Read my [answer](https://stackoverflow.com/a/46861716/4832499) to see all that you never wanted to know about destructors :P – Passer By Oct 21 '17 at 09:09
  • @PasserBy "_An object's lifetime is considered over at the start of the destructor_" which means that lifetime is not a useful C++ concept and should probably be removed – curiousguy Oct 23 '17 at 02:17
  • @curiousguy _"The properties ascribed to objects and references throughout this document apply for a given object or reference only during its lifetime."_ Even though there are exceptions in constructors/destructors, lifetimes still have meaning – Passer By Oct 23 '17 at 06:25
  • @PasserBy "_lifetimes still have meaning_" When the dtor of the complete object begins running, lifetime ends, but at that point exactly nothing changes in practice! – curiousguy Oct 23 '17 at 12:55