-1

I'm working on an application that has the following problem. Basically there are a lot of object that implicitly might destroy themselves. 

void Foo::func()
{
    ...
    Bar b;
    b.func2();
}

Here the func2 might destroy the foo object calling it. Since it is not very obvious that this might happen I would like to make sure that no member variables of the foo object can be accessed after this call, since I cannot guarantee they are still valid. If this call to b is the last call I am perfectly fine, but since I am not the only one working on this project and the destruction is not obvious I would like to implement something to block usage this after these calls. Is there a way to implement this without completely refactoring the current design (since it is use extensively thoughout the project)?

Some background info:

It is a Qt UI application that uses QML to keep track of a stack of "screens" (a screen is a QML + its corresponding C++ model). The Bar class in my example is the 'stack manager' which controls the lifetime of the screens. It is a singleton (I know). The Foo class is a C++ model for a specific QML. The function Foo::func() is a function that can be called either from user input in the QML or another system event. The function normally processes the event, but occasionaly it can tell the screen manager to delete one or more screen(s), which in turn deletes the model corresponding to that screen (which might be the calling model).

Frank
  • 2,446
  • 7
  • 33
  • 67
  • 1
    Perhaps introduce the concept of *ownership*, especially *shared* ownership, using [`std::shared_ptr`](http://en.cppreference.com/w/cpp/memory/shared_ptr). Possibly with the help of [`std::enable_shared_from_this`](http://en.cppreference.com/w/cpp/memory/enable_shared_from_this). – Some programmer dude May 08 '18 at 08:48
  • 2
    Unless you add a layer of "pseudo-destructedness", `this` will be in an invalid state after destruction and you cannot rely on anything about it. That would be a "no" to your question. Which design could best accomodate your needs for as little refactoring and as much safety as possible is hard to tell from the information available in the question. – Max Langhof May 08 '18 at 08:58
  • 2
    The short answer is that `b.func2()` should not destroy `b`, because that is a dreadful design decision. Doing so, in your use case, causes the caller `Foo::func()` to have undefined behaviour, since it may destroy `b` a second time (as it passes out of scope). Instead, return a value which indicates that THE CALLER should stop using `b` and allow its life to end (e.g. by returning in your example). – Peter May 08 '18 at 09:18
  • 1
    @Peter `b.func2()` would destroy `foo` (of type `Foo`) according to the question, not `b`. – Max Langhof May 08 '18 at 10:46
  • 2
    @Frank A critical piece of information that is missing in the question is how `b.func2` would know about the `foo` that it is called from. As written, there must be some global state, because otherwise `b` has no clue which or how many `Foo` objects exist (what would happen if you called `b.func2()` from outside a `Foo`?). – Max Langhof May 08 '18 at 10:50
  • 1
    Does this question help with your issue? https://stackoverflow.com/questions/4888189/how-delete-and-deletelater-works-with-regards-to-signals-and-slots-in-qt – Jay May 08 '18 at 16:14

1 Answers1

6

Serious design problem ?

The this pointer can't be blocked. As with all raw pointers, you must be sure that it's valid before dereferencing it.

But that's not all: what if the Foo that was calling fun2() was a local object of automatic storage duration ?

  • If you'd destroy the object in the function call by invoking its destructor, you'd have to ensure that there's still a valid object at the same place: otherwise the automatic invocation of the destructor when leaving the block in which the object was defined might lead to undefined behavior.
  • If you'd delete the object instead of just destroying it, it would be undefined behavior in any case.

Alternative design with smart pointers

One way out of this situation would be to create all the Foo and Bar objects using factories (making sure that the constructor is not public), and let these factories return shared_ptr.

This has the advantage that if an object is not used anymore, it will destroy itself. That's a lot cleaner than managing destruction manually.

You'd face however several challenges:

  • You'd have to refactor all the code to replace raw pointers by smart pointers
  • In order to avoid endless live due to cross referencing (two objects keeping each a slart pointer to the other), you'd need to figure out when to use shared_ptr (if the pointer must ensure the object is still alive) and when to use weak_ptr (i.e. pointer that doesn't contribute to keeping objects alive).

Alternative design with states

Another alternative would be to use states to make the difference between an active object that can be used, and a deceased objet that shall no longer be used. So instead of deleting the object, you'd call a state change function that would clean/reset the object.

This can be implemented :

  • either very nicely with a state design pattern (but this could require some extensive refactoring),
  • or with a simple flag. It would then be checked at the beginning of each member function, and before accessing data members that follows a call that could potentially change the object's state. This is still error prone and requires analysis, but would require mostly "local" changes and gives thus more flexibility for refactoring.

The advantage of this approach is that the state change function will be very simple (most of the things you have currently in the destructor, but avoiding they could be done by accident a second time) and make the tricky part very visible.

The difficulty will be to ensure that the deceased objects are cleaned for good. So you need to address the ownership issue that was already mentioned earlier, and make sure the objects are not deleted in the middle of their actions.

Community
  • 1
  • 1
Christophe
  • 68,716
  • 7
  • 72
  • 138