Subscriptions for destructions and notifications

[mxgrey]

This pull request introduces a subscription concept, aimed primarily at Entities (but extendable to anything that inherits the Subscription class).

The motivation behind this change is that, in a complex framework, it can be difficult (or impossible) to know whether a pointer that you are holding is still valid. If an object is deleted in one part of the code, but another part of the code is holding on to a pointer to that object, there might not be an easy way to communicate that the pointer is now invalid. Additionally, when changes are made to an object in one part of the code, it might be useful to receive a notification about it in another part of the code (similar to the already existing auto-updating features, but more extensible).

To get these features, three new classes are introduced in this pull request: Subscription, Subscriber, and sub_ptr.

Subscription is a base class that is able to send out notifications. Any class that inherits Subscription can be subscribed to. A class that inherits Subscription will always send out a destruction notification upon destruction, so all of its Subscribers will be informed when it gets deleted. Inheriting classes also have the ability to send out arbitrary notifications which are simply integers. The inheriting class is expected to enumerate the meanings of its integer notifications.

Subscriber is a base class that facilitates receiving and managing subscriptions. It offers virtual functions that can be overridden in order to customize the handling of incoming notifications.

sub_ptr is a templated "subscribed" smart pointer that works on any class that inherits Subscription. It allows the user to create a pointer to a Subscription-inheriting class, and that pointer will turn to a nullptr when the class it points to is deleted (no matter where or how it happens). sub_ptr will also keep track of the very last notification sent out by the Subscription.

There is no overhead added by the new features in this pull request, except for what the end-users themselves decide to create. If the Subscription model goes unused, it will perform no extra work.

Why not reference-counting / shared pointers?

This Subscription model is not a memory management tool. It will not help to prevent memory leaks; it only helps to prevent segfaults that could be caused by dereferencing an invalidated pointer. While I do think that DART could benefit greatly from more rigorous memory management, I do not believe that reference-counting or shared pointers are the right solution, because the resources in DART have a very rigid hierarchy of ownership (e.g. Joints are owned by their child BodyNode; BodyNodes are owned by their parent Skeletons; and Skeletons are owned by their World), and reference-counting or shared pointers would undermine that hierarchy. I do have a proposal to help with memory management in DART, but it would be unrelated to this pull request.

[mxgrey]

As a point of comparison, sub_ptr is conceptually similar to the std::weak_ptr model, except sub_ptr is even weaker (it does not have ownership of the object at any time, even temporarily), and sub_ptr does not rely at all on a shared_ptr framework.

[jslee02]

Change the copyright year to 2015. The beginning year should be the year of the code was initially created, and the ending year should be the year of the release of DART.

[jslee02]

Regarding destruction notifications, I'd like to propose smart pointer approach instead of using class inheritance.

I pasted the original motivation to remind:

The motivation behind this change is that, in a complex framework, it can be difficult (or impossible) to know whether a pointer that you are holding is still valid. If an object is deleted in one part of the code, but another part of the code is holding on to a pointer to that object, there might not be an easy way to communicate that the pointer is now invalid.

This means we need a way to observe whether the holding object is valid or not.

Here is more specific example with code:

class Skeleton
{
public:
  Skeleton() : mBodyNode(new BodyNode()) {}
  ~Skeleton() { delete mBodyNode; }  // mBodyNode is destructed here
  BodyNode* getBodyNode() { return mBodyNode; }
protected:
  BodyNode* mBodyNode;
};

void bar(BodyNode* _bodyNode)
{
  std::cout << "My name is " << _bodyNode->mName << std::endl;
}

void foo()
{
  Skeleton* skel = new Skeleton();
  BodyNode* body = skel->getBodyNode();
  bar(body);  // okay

  delete skel;  // the object 'body' is pointing is destroyed here, but 'body' doesn't know it
  bar(body);  // segfault!
}

There already exist smart pointers in STL (std::shared_ptr, std::weak_ptr, and std::unique_ptr). The proposing new smart pointers, however, fulfill all the requirements. Here I summarized the requirements to solve our problem.

Requirements

(a) A Skeleton has BodyNodes pointer and is responsible to the creation and destruction. The ownership shouldn't be shared.
(b) A Skeleton should be able to return BodyNode pointer to others but shouldn't share the ownership.
(c) The returned BodyNode pointer is not allowed to destruct the actually object.
(d) The returned BodyNode pointer should be able to check if the pointer is valid or not.

Possible solution

A. shared_ptr and weak_ptr
Let's assume that Skeleton stores BodyNode pointer as shared_ptr and returns it as weak_ptr. Then (b), (c), and (d) will be solved. It seems (a) is also solvable if Skeletons never return BodyNode pointer as share_ptr but it's not. Here is an example:

std::weak_ptr<BodyNode> weakBody = skel->getBodyNode();  // still skel has the ownership with reference count 1

// to do actual work with weakBody it needs to converted share_ptr
std::shared_ptr<BodyNode> shareBody = weakBody.lock();  // now the ownership is shared !!

B. unique_ptr
If Skeleton store BodyNode pointer with unique_ptr, there is no way to return BodyNode pointer holding the ownership. We ought to transfer the ownership or return raw pointer.

C. sub_ptr and ob_ptr (proposing)
I'd like to introduce two classes (subject_pointer (a.k.a. sub_ptr) and observer_pointer (a.k.a. ob_ptr) ) that take the middle ground between A and B. sub_ptr is similar to unique_ptr in terms of single ownership, and ob_ptr is similar to weak_ptr in terms of shared observation.

	shared ownership	shared observation
unique_ptr	X	X
sub_ptr and ob_ptr	X	O
shared_ptr and weak_ptr	O	O

Here is a example code that utilizes the new classes:

struct BodyNode { std::string mName; }
class Skeleton
{
public:
  Skeleton() : mBodyNode(new BodyNode()) {}
  ~Skeleton() {}  // mBodyNode will be automatically destroyed if assigned
  ob_ptr<BodyNode> getBodyNode() { return mBodyNode; }
protected:
  sub_ptr<BodyNode> mBodyNode;
};

void bar(ob_ptr<BodyNode> _bodyNode)
{
  if (_bodyNode.valid())
    std::cout << "My name is " << _bodyNode->mName << std::endl;
  else
    std::cout << "I'm nobody." << std::endl;
}

void foo()
{
  Skeleton* skel = new Skeleton();
  ob_ptr<BodyNode> body = skel->getBodyNode();
  bar(body);  // okay

  delete skel;  // the object 'body' is pointing is destroyed here, but still 'body' can handle it
  bar(body);  // okay, will print "I'm nobody."
}

Now (a), (b), (c), (d) are all satisfied.

This smart pointer approach can be applied to any class without class inheritance. So it's more general solution while still solving the target problem (to prevent segfaults that could be caused by dereferencing an invalidated pointer).

Please let me know what do you think.

[mxgrey]

To me this actually sounds like a more intensive structural change than the inheritance approach.

If we take this approach, we'll need to design a very specialized reference-counting pointer (which could be a challenge when it comes to threading) and then modify every pointer-returning function to return an ob_ptr value (and there are many pointer-returning functions). Also, this would not allow the custom handling of object destructions. For example, in my osgDart framework, I have mappings from DART objects to OSG objects, and I want the OSG objects and their mappings to be destroyed as soon as the DART object that they correspond to is destroyed. This is easily done by overriding a virtual function using the Subscriber class.

If I understand correctly, the motivation for avoiding inheritance is to offer greater usability by allowing the method to work on any arbitrary class. But I don't think this motivation is truly met, because this method still requires that a very particular set of rules needs to be followed by any object that wants to benefit from it. It foregoes the inheritance requirement by imposing a greater number of code management requirements (e.g. using sub_ptr internally and returning ob_ptr in every single pointer-returning function).

We'll mostly want to get destruction notifications for classes that we have complete control over, in which case adding : public Publisher to the class definition is trivial. And even if we want a third-party class to benefit from it, it's not hard to extend it. Suppose Foo is a class from a third-party library, then all we have to do is:

class PublishingFoo : public Foo, public Publisher
{
public:
  using Foo::Foo; // inherit Foo constructor
};

Then the PublishingFoo class offers everything the Foo class does, plus the destruction notifications, all without needing to worry about code management. This seems much easier and usable to me than intertwining specialized pointer classes throughout the API.

(I'm only using the terms "Publisher" and "Subscriber" because I still haven't thought of better terms for them, but I definitely want to rename them.)

[jslee02]

I want the OSG objects and their mappings to be destroyed as soon as the DART object that they correspond to is destroyed.

Indeed, if the destruction notification should be sent as soon as the object is destructed, then ob_ptr doesn't fit.

I have one concern then. Is it possible that a class can be a publisher and a subscriber at the same time without any difficulties?

[mxgrey]

Yep, any class can definitely be both with no problems. In fact, I've already been doing that.

I just thought of one potential issue with the inheritance approach: the diamond problem. But that's easily solved; we just need make sure we always virtually inherit the Publisher or Subscriber class. Since both Publisher and Subscriber only have a default constructor, we don't need to worry about any virtual inheritance construction complications.

Also, I'm thinking of changing the class names to Subject (for Publisher) and Observer (for Subscriber), because I do like those terms.

[jslee02]

Subject will support general notifications also, or just destruction notifications?

[mxgrey]

It would just support destruction notifications.

[jslee02]

If Subscriber is changed to Observer then sub_ptr would be better with new name pertain to observer. Something like ob_ptr or obs_ptr.

[mxgrey]

I actually think sub_ptr still works, because it would be a pointer to a Subject, therefore sub_ptr.

[jslee02]

I was confused because it inherits Observer. You're right, it points Subjects so sub_ptr.