Best Practice for Scoped Reference Idiom?

I just got burned by a bug that is partially due to my lack of understanding, and partially due to what I think is suboptimal design in our codebase. I'm curious as to how my 5-minute solution can be improved.

We're using ref-counted objects, where we have AddRef() and Release() on objects of these classes. One particular object is derived from the ref-count object, but a common function to get an instance of these objects (GetExisting) hides an AddRef() within itself without advertising that it is doing so. This necessitates doing a Release at the end of the functional block to free the hidden ref, but a developer who didn't inspect the implementation of GetExisting() wouldn't know that, and someone who forgets to add a Release at the end of the function (say, during a mad dash of bug-fixing crunch time) leaks objects. This, of course, was my burn.

void SomeFunction(ProgramStateInfo *P)
{
   ThreadClass *thread = ThreadClass::GetExisting( P );
   // some code goes here
   bool result = UseThreadSomehow(thread);
   // some code goes here
   thread->Release();  // Need to do this because GetExisting() calls AddRef()
}

So I wrote up a little class to avoid the need for the Release() at the end of these functions.

class ThreadContainer
{
private:
    ThreadClass *m_T;
public:
    ThreadContainer(Thread *T){ m_T = T; }
    ~ThreadContainer() { if(m_T) m_T->Release(); }
    ThreadClass * Thread() const { return m_T; }
};

So that now I can just do this:

void SomeFunction(ProgramStateInfo *P)
{
   ThreadContainer ThreadC(ThreadClass::GetExisting( P ));
   // some code goes here
   bool result = UseThreadSomehow(ThreadC.Thread());
   // some code goes here
   // Automagic Release() in ThreadC Destructor!!!
}

What I don't like is that to access the thread pointer, I have to call a member function of ThreadContainer, Thread(). Is there some clever way that I can clean that up so that it's syntactically prettier, or would anything like that obscure the meaning of the container and introduce new problems for developers unfamiliar with the code?

Thanks.

From stackoverflow

• Take a look at ScopeGuard. It allows syntax like this (shamelessly stolen from that link):

  {
      FILE* topSecret = fopen("cia.txt");
      ON_BLOCK_EXIT(std::fclose, topSecret);
      ... use topSecret ...
  } // topSecret automagically closed
  

  Or you could try Boost::ScopeExit:

  void World::addPerson(Person const& aPerson) {
      bool commit = false;
      m_persons.push_back(aPerson);  // (1) direct action
      BOOST_SCOPE_EXIT( (&commit)(&m_persons) )
      {
          if(!commit)
              m_persons.pop_back(); // (2) rollback action
      } BOOST_SCOPE_EXIT_END
  
      // ...                        // (3) other operations
  
      commit = true;                // (4) turn all rollback actions into no-op
  }
  

• Yes, you can implement operator ->() for the class, which will recursively call operator ->() on whatever you return:

  class ThreadContainer
  {
  private:
      ThreadClass *m_T;
  public:
      ThreadContainer(Thread *T){ m_T = T; }
      ~ThreadContainer() { if(m_T) m_T->Release(); }
      ThreadClass * operator -> () const { return m_T; }
  };
  

  It's effectively using smart pointer semantics for your wrapper class:

  Thread *t =  new Thread();
  ...
  ThreadContainer tc(t);
  ...
  tc->SomeThreadFunction(); // invokes tc->t->SomeThreadFunction() behind the scenes...
  

  You could also write a conversion function to enable your UseThreadSomehow(ThreadContainer tc) type calls in a similar way.

  If Boost is an option, I think you can set up a shared_ptr to act as a smart reference as well.

  Furious Coder : What does that look like in use?

  Furious Coder : Ah, I get it. That calls functions on the hidden member (which IS useful, so thanks), but I guess what I want is a cleaner way to pass the hidden member INTO functions, perhaps by aliasing the container class as the hidden class. I guess I could overload () and have that return m_T...

  mwigdahl : No, that wouldn't work. You'll need an automatic conversion from ThreadContainer to Thread *, and that's possible, but can get tricky. Really, I'd suggest looking at bb's Boost suggestion below if you are able to use Boost. shared_ptr sounds like exactly what you're looking for.

  mwigdahl : And the nice thing about Boost is that it's already heavily debugged for exactly the kind of substitutive use you want...

• use boost::shared_ptr it is possible to define your own destructor function, such us in next example: http://www.boost.org/doc/libs/1_38_0/libs/smart_ptr/sp_techniques.html#com

  Furious Coder : Mark Ransom's answer was what I was going for, but your answer is pushing me down a good direction of learning about new stuff and a higher level solution to the overall problem, so I'm going to mark it as "The" answer. Thanks.

  bb : Glad to hear. You're welcome. It is really better use one genreic class for all similar things instead write separeted solution for each one.

• You can add an automatic type-cast operator to return your raw pointer. This approach is used by Microsoft's CString class to give easy access to the underlying character buffer, and I've always found it handy. There might be some unpleasant surprises to be discovered with this method, as in any time you have an implicit conversion, but I haven't run across any.

  class ThreadContainer
  {
  private:
      ThreadClass *m_T;
  public:
      ThreadContainer(Thread *T){ m_T = T; }
      ~ThreadContainer() { if(m_T) m_T->Release(); }
      operator ThreadClass *() const { return m_T; }
  };
  
  void SomeFunction(ProgramStateInfo *P)
  {
     ThreadContainer ThreadC(ThreadClass::GetExisting( P ));
     // some code goes here
     bool result = UseThreadSomehow(ThreadC);
     // some code goes here
     // Automagic Release() in ThreadC Destructor!!!
  }
  

  Furious Coder : Ahhh, this is kinda what I was looking for, I just wasn't sure how to do it.

• I would recommend following bb advice and using boost::shared_ptr<>. If boost is not an option, you can take a look at std::auto_ptr<>, which is simple and probably addresses most of your needs. Take into consideration that the std::auto_ptr has special move semantics that you probably don't want to mimic.

  The approach is providing both the * and -> operators together with a getter (for the raw pointer) and a release operation in case you want to release control of the inner object.