3

While working on a project, I came across an interesting problem when passing an object into another object through its constructor when the object passed in is guaranteed to outlive (in terms of memory lifetime) the recipient object. Please bare in mind that I am still learning the ins-and-outs of C++11/C++14, so I am looking for constructive discussion that will help in my understanding of memory management and lifetimes with C++11/C++14-style semantics.

The setup for this question is as follows:

class TopLevelClass {
public:
    void someMethod (int someValue) {
        // Do some work
    }

    std::unique_ptr<Context> getContext () {
        return std::make_unique<Context>(this);
    }
};

class Context {
public:
    Context (TopLevelClass* tlc) : _tlc(tlc) {}

    void call (int value) {
        // Perform some work and then call the top level class...
        _tlc->someMethod(value);
    }

protected:
    TopLevelClass* _tlc;
};

Although a valid alternative to this setup would be to pass the TopLevelClass as an argument into the call method of the Context class, this is not possible in the scenario I am illustrating: The client code with access to a Context object may not have access to the TopLevelClass object.

While the code illustrated above is functionality correct for my needs, I feel as though there exists a code smell. Namely, storing a handle to the TopLevelClass object as a raw pointer does not convey the fact that the Context class is not responsible for managing the lifetime of this pointer (since, in this case, the TopLevelClass is guaranteed to outlive any Context object). Additionally, with use of C++11, I am hesitant to use a raw pointer, rather than a smart pointer (as per Scott Meyer's suggestion in Effective Modern C++).

One alternative I explored is to pass in the handle to the TopLevelClass using a shared pointer and storing this handle within the Context class as a shared pointer. This requires that the TopLevelClass inherit from std::enabled_shared_from_this in the following manner:

class TopLevelClass : public std::enable_shared_from_this<TopLevelClass> {
public:
    // Same "someMethod(int)" as before...

    std::unique_ptr<Context> getContext () {
        return std::make_unique<Context>(shared_from_this());
    }
};

class Context {
public:
    Context (std::shared_ptr<TopLevelClass> tlc) : _tlc(tlc) {}

    // Same "call(int)" as before...

protected:
    std::shared_ptr<TopLevelClass> _tlc;
};

The downside to this approach is that unless a std::shared_ptr exists for TopLevelClass a priori, then a std::bad_weak_ptr exception will be thrown (for more information, see this post). Since, in my case, there is no std::shared_ptr<TopLevelClass> created in the code, I cannot use the std::enable_shared_from_this<T> approach: I am restricted to returning a single instance of TopLevelClass using a static raw pointer, as per the requirements of my project, as follows

static TopLevelClass* getTopLevelClass () {
    return new TopLevelClass();
}

Is there an approach that exists that conveys the fact that Context is not responsible for managing its handle to the TopLevelClass instance, since the TopLevelClass will be guaranteed to outlive any Context object? I am also open to suggestions about changing the design that will side-skirt the problem altogether, so long as the design change does not overly complicate the simplicity of the design above (i.e., creating many different classes in order to get around simply passing a single pointer into the constructor of Context).

Thank you for your help.

Community
  • 1
  • 1
Justin Albano
  • 3,809
  • 2
  • 24
  • 51
  • 3
    Instead of passing a pointer, pass and store a reference. – clcto Nov 06 '15 at 21:03
  • Is there any reason you can't wrap the getTopLevelClass function? If there is, then go with a reference. – Anon Mail Nov 06 '15 at 21:05
  • 3
    `getTopLevelClass()` is evil. From the signature you might expect it to return a singleton but actually it returns an owning raw pointer to a new object each time you call it that the caller has to remember to delete! – Chris Drew Nov 06 '15 at 22:02
  • Yes, I am restricted by the library that I hook into (by contract for the project I am developing for). The library expects that static method to be implemented and it calls it to obtain the `TopLevelClass` object. I personally don't like that this is how the library is requiring the creation of a single single instance of the `TopLevelClass`, but I'm required to support that interface. I think using a reference is another good option, but at the same time, it does not explicitly convey the ownership semantics (i.e., in the same manner that a `std::unique_ptr` or `std::shared_ptr` does). – Justin Albano Nov 06 '15 at 22:08
  • @ChrisDrew I agree. I am restricted from changing this (the library I use requires that this static method be implemented; this is the mechanism I am required to interface with the library through). I agree, though: The static method approach is very restrictive. In the case of my project, that static method is only called once by the library, but that is not explicitly stated by the semantics of that method (the library is C++98 compliant, so it does not return something akin to a `std::unique_ptr`). – Justin Albano Nov 06 '15 at 22:12
  • 1
    Does the library explicitely specify the implementation of the function or just the signature? Otherwise use `TopLevelClass *getTopLevelClass() { static TopLevelClass tlc; return &tlc; }` – villintehaspam Nov 06 '15 at 22:28
  • 1
    @unseenghost does the library delete the `TopLevelClass` at the end? If it doesn't then you could write `static TopLevelClass* getTopLevelClass(){ static TopLevelClass tlc; return &tlc;}` and still implement the same interface without owning raw pointers. – Chris Drew Nov 06 '15 at 22:30
  • @villintehaspam, wow, I didn't copy you, honest :) – Chris Drew Nov 06 '15 at 22:32
  • @villintehaspam @ChrisDrew I do not know if it explicitly `delete`s the pointer, but I presume it does, as the library does not mention that I am responsible for deleting the pointer (this is exactly the issue I am trying to avoid in my question: Who owns the pointer? Who is responsible for deleting it? etc.). I do not have the code in front of me at the moment, but I could check the source code to see if it does. PS: @Chris Drew Creepy, I don't know if I believe that (j/k) – Justin Albano Nov 06 '15 at 22:34
  • @unseenghost what is the library can I ask? – Chris Drew Nov 06 '15 at 22:39
  • 1
    @ChrisDrew: Great minds... :) Good point though that it must be clear whether or not the library deletes the pointer or not. If it is a well designed library, then IMHO it should not delete the pointer, because hey, it is a raw pointer, i.e. it is non-owning... – villintehaspam Nov 06 '15 at 22:39
  • 2
    Scott Meyers does _not_ say to never use raw pointers. E.g here is a quote from Effective Modern C++, item 20, p138: `Back-links from children to parents can be safely implemented as raw pointers, because a child node should never have a lifetime longer than its parent. There’s thus no risk of a child node dereferencing a dangling parent pointer.` – Chris Drew Nov 06 '15 at 23:19
  • Agreed. I don't believe in *nevers* or *always* with programming (hence my personal hesitation, but not downright objection). I've heard too many dogmatic statements like, "This is alwayer better" or "never use that." Thanks for the quote. As for the library, unfornately, it's proprietary, so it is not openly available. – Justin Albano Nov 06 '15 at 23:25
  • 1
    A pragmatic alternative might be `static TopLevelClass* getTopLevelClass(){ static TopLevelClass *tlc = new TopLevelClass(); return tlc;}`. Technically it is a memory leak but not a real one as it only leaks one object during the whole run of the program that will be cleared up when the program closes anyway. And if doesn't matter if the library _does_ decide to delete it. This is actually an approach recommended in the [C++FAQ](https://isocpp.org/wiki/faq/ctors#static-init-order-on-first-use) – Chris Drew Nov 06 '15 at 23:48
  • Clever approach. I will look into it. Thanks for the help. – Justin Albano Nov 06 '15 at 23:53

2 Answers2

3

Passing in a raw pointer the way you are doing absolutely should mean that no ownership is being transferred.

If you have heard somone saying "don't use raw pointers" you probably missed part of the sentence - it should be "don't use owning raw pointers", i.e. there should not be a place somewhere where you have a raw pointer that you need to call delete on. Except possibly in some low level code. There is absolutely nothing wrong with just passing in pointers if you know for a fact that the object being pointed to outlives the object getting the pointer.

You are saying "Namely, storing a handle to the TopLevelClass object as a raw pointer does not convey the fact that the Context class is not responsible for managing the lifetime of this pointer". On the contrary, storing a raw pointer means exactly this - "This object does not manage the lifetime of the object pointed to by this pointer". In C++98 style code it did not necessarily mean that though.

An alternative to using a pointer is to use a reference. There are some caveats to this though, as you must initialize it in the constructor for instance and it cannot be set to nullptr like a pointer (which can also be a good thing). I.e:

class TopLevelClass {
public:
    void someMethod (int someValue) {
        // Do some work
    }

    std::unique_ptr<Context> getContext () {
        return std::make_unique<Context>(*this);
    }
};

class Context {
public:
  Context(TopLevelClass &tlc) : _tlc(tlc) {}

  void call (int value) {
    // Perform some work and then call the top level class...
    _tlc.someMethod(value);
  }

private:
  TopLevelClass &_tlc;
};

Here are some articles on the topic:

The C++ Core Guidelines:

https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rr-ptr

Some earlier articles by Herb Sutter:

http://herbsutter.com/2013/05/29/gotw-89-solution-smart-pointers/

http://herbsutter.com/2013/05/30/gotw-90-solution-factories/

http://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/

http://herbsutter.com/elements-of-modern-c-style/

There are probably also lots of videos from CppCon as well as Cpp and Beyond, but I was a bit too lazy to google up the appropriate ones.

villintehaspam
  • 8,540
  • 6
  • 45
  • 76
  • Thank you. I agree that raw pointers are not forbidden, but I disagree about the semantics. In my example, I explicitly state that the `TopLevelClass` object will outlive the `Context` object, but that is not stated explicitly in the code (I make a differentiation between the semantics of the language and simply writing a comment). In general, there are a multitude of issues with raw pointers semantics: Am I responsible for deleting the object? If so, how so (`delete` or `delete[]`)? will it ever dangle? For my "don't use raw pointers" comment, see [here](http://goo.gl/xVn5gI), Ch. 4). Thanks! – Justin Albano Nov 06 '15 at 22:28
  • 1
    @unseenghost you are never responsible for deleting non-owning raw pointers. – Chris Drew Nov 06 '15 at 22:36
  • I agree. The intent is that I explicitly state that I do not own the pointer. A raw pointer does not convey that with the same rigidity that a `std::unique_ptr` would for unique ownership, for example. I.e., I know that a `std::unique_ptr` explicitly conveys unique ownership. – Justin Albano Nov 06 '15 at 22:41
  • 1
    @unseenghost: That is my point really. If you have a raw pointer, it means "this is a non-owning pointer". Whoever provided that pointer to the object is responsible for making sure that it does not dangle. If you cannot guarantee that it doesn't outlive the object, then you cannot use raw pointers by themselves - but that is another topic and a bit too large for comments I think. – villintehaspam Nov 06 '15 at 22:44
  • 1
    Don't take my word for it, take Bjarne Stroustrup and Herb Sutters and probably a lot more. I don't have the Scott Meyers book, but I am pretty sure that he agrees on this topic. – villintehaspam Nov 06 '15 at 22:45
  • Gotcha. I appreciate your help. I wanted to see if there was a more explicit way to declare the intent. Believe me, I learn a lot from these discussions. I appreciate it. – Justin Albano Nov 06 '15 at 23:27
1

One option is to use a type definition to convey non-ownership:

#include <memory>

template<typename T>
using borrowed_ptr = T *;

class TopLevelClass;

class Context {
public:
  Context(borrowed_ptr<TopLevelClass> tlc)
    : _tlc(std::move(tlc))
  { }

private:
  borrowed_ptr<TopLevelClass> _tlc;
};

class TopLevelClass {
public:
  std::unique_ptr<Context> getContext() {
    return std::make_unique<Context>(this);
  }
};

This cleanly expressed the intent, though _tlc is still convertible directly to a raw pointer. We could make an actual class called borrowed_ptr (similar to shared_ptr) that better hides the raw pointer, but it seems like overkill in this case.

Chris Hayden
  • 1,104
  • 6
  • 6
  • I like how clean this approach is while explicitly stating the non-ownership intent. As a side question, why was a `std::move` used on what amounts to a raw pointer (the underlying type)? – Justin Albano Nov 06 '15 at 21:33
  • 1
    The normal assumption (at least in newer code) should probably be that at raw pointer means "non-owning". Instead, it is probably better to tag any owning raw pointers with an "owner" the way you tag a "borrowed_ptr" (and easier, since there should likely be fewer instances). This is also the approach taken by the Guidelines Support Library (https://github.com/Microsoft/GSL) brought forward at this year's CppCon by Bjarne Stroustrup et al. – villintehaspam Nov 06 '15 at 22:23
  • I'm aware of the guidelines that say a raw pointer should be treated as a non-owning pointer. I just don't agree with them. We go to all this trouble to be expressive in our code, but in this case we are saying just assume that a raw pointer is a non-owning pointer. Why not make it explicit? We can have both "owner" (or "owned_ptr") and have "borrowed_ptr", so this seems like a case where we can have our cake and eat it, too. Also, as you point out, the assumption only applies in new code developed. That leaves out all existing code, which doesn't seem ideal. – Chris Hayden Nov 07 '15 at 18:17
  • 1
    @unseenghost The move in this case doesn't do anything since pointers are primitive types. I've just gotten myself into the habit of always moving non-lvalue reference constructor arguments into place. It does no harm and may help if someone changes the member/constructor parameter types to something non-primitive. – Chris Hayden Nov 07 '15 at 18:21
  • @Chris Hayden Sounds like a good idea. Just wanted to make sure there wasn't something I was missing. Thanks for the help! – Justin Albano Nov 07 '15 at 20:15
  • There is an [`observer_ptr`](http://en.cppreference.com/w/cpp/experimental/observer_ptr) in the Library Fundamentals TS v2, which is a non-owning pointer with a smart-pointer-like interface. – Ilya Popov Feb 02 '16 at 18:21