0

I'm converting some old C++ code to use shared_ptr, unique_ptr and weak_ptr, and I keep running into design problems.

I have "generator" methods that return new objects, and accessor methods that return pointers to existing objects. At first glance the solution seems simple; return shared_ptr for new objects, and weak_ptr for accessors.

shared_ptr completely avoids dangling pointers, since if the object is ever deleted all of its shared and weak pointers know about it. But I keep running into cases where I'm not sure if there are cyclic references among my shared pointers. There are many classes and some of them point at each other; is it possible that at some point a cycle formed? The code is sufficiently complex that it's hard to tell - new classes are being created from instructions in a script file. So I don't know if shared_ptr is actually preventing memory leaks and have been manually deleting all objects, which seems to defeat the point.

I considered using unique_ptr instead, since I don't actually need shared ownership anywhere. (The old C++ code certainly didn't have any shared ownership, it's raw pointers only.) But I can't make weak_ptrs from a unique_ptr, so I have to use raw pointers as stand-ins for weak pointers. This solves the memory leak problem, but I can be left with dangling pointers when the unique_ptr is destroyed.

So it seems that I can have one or the other: bulletproof memory leak prevention or bulletproof dangling pointer prevention, but not both.

People have told me I need to keep the entire program structure in my head so I can verify there are no shared pointer cycles, but that seems error prone. My head is, after all, only so big. Is there a way to achieve memory safety while only needing to consider local code?

To me, that is the central tenement of OO programming, and it seems I have lost it in this case.

Ebonair
  • 221
  • 1
  • 11
  • See this: https://stackoverflow.com/questions/18525845/how-can-i-enforce-single-ownership-with-weak-ptr-or-how-to-subclass-shared-pt – Anton Savin Jan 19 '18 at 23:14
  • 2
    Memory safety is not ensured by slapping `special_pointer_t` on everything. Memory safety is ensured by avoiding pointers whenever possible and using values instead and having a clear understanding what owns what. – nwp Jan 19 '18 at 23:14
  • @AntonSavin I had been considering something like that, but was thinking more about creating a custom unique_ptr with weak references, rather than a shared_ptr without sharing. I suppose they're both roughly equivalent. – Ebonair Jan 19 '18 at 23:20
  • Resource management is a problem with all programming, not just OO. Does the old code work correctly? If yes, probably best to leave it alone. If no, sit down and map out everyone's responsibilities and rewrite the code in accordance to that. What you have was written with a particular memory management idiom in mind and it may not lend itself well to what you have in mind. For example, if it has a problem with dangling pointers, I'd recommend sorting out why the references are dangling in the first place rather than slapping on a bandaid. – user4581301 Jan 19 '18 at 23:30
  • the corollary of a unique_ptr is an observer_ptr or reference_wrapper – Richard Hodges Jan 19 '18 at 23:34
  • One quick way to see if you might have problems with cyclic references: run your program under valgrind (and make sure that your program exits is a controlled fashion, e.g. by returning from main()). If, when your program exits, valgrind reports that memory has been leaked, investigate why, and fix it if possible. Repeat until valgrind reports no memory leaks -- at that point you're in a pretty good state. – Jeremy Friesner Jan 21 '18 at 01:45

1 Answers1

2

A strategy that may work for you is to ensure that all the shared pointers in all of your managed objects are const.

Since a const shared_ptr field can only be assigned when it is constructed, this ensures that the objects can only hold shared pointers to objects that were created before they were. (OK, there are ways around that, but you're not going to do it by mistake)

Since "created before" is a total ordering, that ensures that the graph of shared pointers is acyclic.

Matt Timmermans
  • 53,709
  • 3
  • 46
  • 87