Back to General and Gameplay Programming

Weird STL container + void pointer behavior? [SOLVED]

General and Gameplay Programming Programming

Started by persil May 29, 2005 10:07 AM

9 comments, last by MaulingMonkey 18 years, 10 months ago

persil

199

Author

May 29, 2005 10:07 AM

**** SEE REPLY BELOW, I SIMPLIFIED THE PROBLEM **** Ok, here's my problem. Since I've been recently converted to STL, I'm using iostream, map and sets. I am currently making a simple handle manager. My goal is to be able to have a handle manager which keeps a list of all allocated pointers and their handles, so that I can always know the reference count and access them afterwards. It differs from a linked list pointer from the fact that you can re-assign it a raw pointer and if it was already allocated in other handles, it won't cause double destroy. Here's the whole header source handle.h, there's no code (cpp).


#ifndef _COMP_HANDLE_H_
#define _COMP_HANDLE_H_

#include <algorithm>
#include <map>
#include <set>
#include <cassert>
#include "smart_pointer_common.h"

namespace components
{

/* Forward declaration.
*/
template < class, template < class > class > class handle;

/* Class: handle_mgr
*/
class handle_mgr
{
public:
  /* Constructor and destructor.
  */
  handle_mgr() {}
  ~handle_mgr() {}
  
  /* Add a reference for a pointer. Return *true* if it was the first one.
  */
  bool add_ref( void * ptr, void * handle )
  {
    // Check whether this is the first handle to this pointer
    bool first = m_lookup.find( ptr ) != m_lookup.end();
    
    // Get list reference
    list_type & list = m_lookup[ ptr ];
    
    // Safety check:
    assert( list.find( handle ) == list.end() );
    
    list.insert( handle );
    
    return first;
  }

  /* Remove a reference to a pointer. Return *true* if it was the last one.
  */
  bool remove_ref( void * ptr, void * handle )
  {
    // Safety check: make sure this pointer reference exists
    assert( m_lookup.find( ptr ) != m_lookup.end() );
    
    // Get list reference
    list_type & list = m_lookup[ ptr ];
    
    // Safety check: make sure this handle already has been added
    assert( list.find( handle ) != list.end() );
    
    // Erase handle reference
    list.erase( handle );
    
    // Check if this was the last reference
    bool last = false;
    if ( list.empty() )
    {
      // If so, erase list too
      m_lookup.erase( ptr );
      last = true;
    }
    
    return last;
  }
  
  /* Remove all references to a pointer, nullifying its contained pointer in the
     process.
  */
  template < class T, class H >
  void remove_all_ref( T * ptr, H * handle )
  {
    // Make sure list exists
    assert( m_lookup.find( ptr ) != m_lookup.end() );
    
    // Get list
    list_type & list = m_lookup[ ptr ];
    
    // Loop through all iterators and nullify them
    for( list_type::iterator i = list.begin(); i != list.end(); ++i)
    {
      reinterpret_cast< H * >( *i )->m_ptr = 0;
    }
    
    // Erase this lookup entry
    m_lookup.erase( ptr );
  }
  
private:
  /* The actual lookup table of pointers.
  */
  typedef std::set < void * > list_type;
  typedef std::map < void *, list_type > lookup_type;
  lookup_type m_lookup;
  
  /* Prohibited operation
  */
  handle_mgr( const handle_mgr & );
};

/* Class: handle
*/
template < class T, template < class > class PointerPolicy = single_pointer_policy >
class handle
  : public smart_pointer_base < T, PointerPolicy >
{
  friend class handle_mgr;
  
public:
  typedef smart_pointer_base<T, PointerPolicy> base;
  typedef typename base::value_type value_type;
  typedef typename base::reference_type reference_type;
  typedef typename base::pointer_type pointer_type;
  typedef typename base::pointer_policy_type pointer_policy_type;
  
  /* Constructor for a null handle.
  */
  handle() : m_mgr( 0 ), m_ptr( 0 ) {}
  
  /* Constructor for a valid handle.
  */
  explicit handle( handle_mgr & mgr, pointer_type ptr ) : m_mgr( &mgr ), m_ptr( ptr )
  {
    _attach();
  }
  
  /* Create a copy of another handle.
  */
  handle( const handle & copy ) : m_mgr( copy.m_mgr ), m_ptr( copy.m_ptr )
  {
    _attach();
  }
  
  /* Copy constructor with automatic conversion.
  */
  template < class T0, template < class > class PP0 >
  handle( const handle<T0, PP0> & copy ) : m_mgr( 0 ), m_ptr( 0 )
  {
    m_mgr = reinterpret_cast< const handle<T, PointerPolicy> & >( copy ).m_mgr;
    m_ptr = copy.get();
  }

  /* Destructor
  */
  ~handle()
  {
    _destroy();
  }
  
  /* Handle copying operator.
  */
  const handle & operator = ( const handle & copy )
  {
    if ( this != © )
    {
      _destroy();
      m_mgr = copy.m_mgr;
      m_ptr = copy.m_ptr;
      _attach();
    }
    return *this;
  }
  
  /* Universal copying operator.
  */
  template < class T0, template < class > class PP0 >
  const handle & operator = ( const handle<T0, PP0> & copy )
  {
    if ( reinterpret_cast<void *>(this) != reinterpret_cast<void *>(©) )
    {
      _destroy();
      m_mgr = reinterpret_cast< const handle<T, PointerPolicy> & >( copy ).m_mgr;
      m_ptr = copy.get();
      _attach();
    }
    return *this;
  }
  
  /* Assign a new pointer. Assume the same handle manager is used.
  */
  const handle & operator = ( pointer_type ptr )
  {
    _destroy();
    m_ptr = ptr;
    _attach();
    return *this;
  }
  
  /* Return the pointer.
  */
  pointer_type get() const
  {
    return m_ptr;
  }
  
  /* Get a hold on the contained pointer and release all responsibilities.
  */
  pointer_type release()
  {
    pointer_type tmp = m_ptr;
    _nullify();
    return tmp;
  }
  
  /* Return contained pointer, for derefence use.
  */
  pointer_type operator -> () const
  {
    return m_ptr;
  }
  
  /* Return a reference to contained object.
  */
  reference_type operator * () const
  {
    return *m_ptr;
  }
  
  /* Returns *true* if a valid pointer is contained.
  */
  operator bool () const
  {
    return static_cast<bool>(m_ptr);
  }

private:
  /* Parent manager.
  */
  handle_mgr * m_mgr;
  
  /* The pointer.
  */
  pointer_type m_ptr;
  
  /* Handle attaching to an already built chain.
  */
  void _attach()
  {
    if( m_ptr )
    {
      assert( m_mgr );
      m_mgr->add_ref( m_ptr, this );
    }
  }
  
  /* Handle the cleaning.
  */
  void _destroy()
  {
    if( m_ptr )
    {
      assert( m_mgr );
      if( m_mgr->remove_ref( m_ptr, this ) )
      {
        pointer_policy_type::destroy( m_ptr );
      }
      m_ptr = 0;
    }
  }
  
  /* Nullify all pointers.
  */
  void _nullify()
  {
    if( m_ptr )
    {
      assert( m_mgr );
    }
  }
};

}

#endif // _COMP_HANDLE_H_^

Now, as soon as the pointer gets inserted in the lookup, cout stops outputting, my guess is that some parts of iostream stop working. For example, I am doing that in my main:


int main()
{
  {
    handle_mgr manager;

    typedef handle<Bob> BH;

    BH test( manager, new Bob() );
  }

  system("PAUSE");
  return 0;
}

Class Bob is a dummy class which outputs to cout its construction/copy/destructions. When compiling the above, I only have the constructor shown, not the destructor. However, the object is correctly destroyed, I have tried using printf() instead, it goes in the destructor, and my memory manager shows no leak. So it's reall cout that's stuck, but why? Please somebody help me :) Oh, yeah, I'm using Microsoft Visual C++ 2003 toolkit to compile this. If you want to compile the above code, there is also a file called smart_pointer_common.h, which is this:


#ifndef _COMP_SMART_POINTER_COMMON_H_
#define _COMP_SMART_POINTER_COMMON_H_

#include "common.h"

namespace components
{

/* Policy for a single pointer.
*/
template < class T >
struct single_pointer_policy
{
  static void destroy( T* ptr ) { delete ptr; }
};

/* Destruction policy for array pointers.
*/
template < class T >
struct array_pointer_policy
{
  static void destroy( T* ptr ) { delete [] ptr; }
};

/* Destruction policy for weak pointers.
*/
template < class T >
struct weak_pointer_policy
{
  static void destroy( T* ptr ) {}
};

/* Base class for smart pointers.
*/
template < class T, template < class > class PointerPolicy >
struct smart_pointer_base
{
  typedef T value_type;
  typedef T& reference_type;
  typedef T* pointer_type;
  typedef PointerPolicy<T> pointer_policy_type;
};

}

#endif // _COMP_SMART_POINTER_COMMON_H_

[Edited by - persil on May 29, 2005 11:13:26 AM]

persil

199

Author

May 29, 2005 10:25 AM

I simplified the problem to a more basic equation and the problem ran away, so, well, it seems to have something to do with the rest of my code, so well, sorry for your time. Guess I'll have to dig deeper.

MaulingMonkey

1,729

May 29, 2005 10:26 AM

#include <iostream>#include <cstdlib>#include "comp_handle.hh"using namespace components;using namespace std;struct Bob {	Bob( void ) {		cout << "cout: Bob()" << endl;		printf( "printf: Bob()\n\r" );	}		Bob( const Bob & other ) {		cout << "cout: Bob( other )" << endl;		printf( "printf: Bob( other )\n\r" );	}		Bob & operator=( const Bob & other ) {		cout << "cout: Bob = other" << endl;		printf( "printf: Bob = other\n\r" );		return *this;	}		~Bob( void ) {		cout << "cout: ~Bob()" << endl;		printf( "printf: ~Bob()\n\r" );	}};int main(){  {    handle_mgr manager;    typedef handle<Bob> BH;    BH test( manager, new Bob() );  }  system("PAUSE");  return 0;}

C:\eclipse\workspace\test.8>Debug\test.8cout: Bob()printf: Bob()cout: ~Bob()printf: ~Bob()Press any key to continue . . .C:\eclipse\workspace\test.8>_

You're saying this is different on your system?

persil

199

Author

May 29, 2005 10:32 AM

Yeah, thanks MaulingMonkey for trying out, I also isolated that part of the code fom the rest of my system and it worked fine, so it must be some weird inter-ops.

BTW, does it seem right, the way I'm implementing the handle idea?

persil

199

Author

May 29, 2005 11:18 AM

Has anyone ever used "Fluid Studios Memory Manager.h" ? Well, it's a nice memory logger and leak detector for C++.

I put the include for this module in my "common.h" while thinking it would end up being the last one everytime. WRONG. I just forgot about that. It was clearly specified in the mmgr's document, that I must put the include after standard includes and before my own includes. Well, quite complicated, but it seems to mess up stuff when not done properly!

That's it.

MaulingMonkey

1,729

May 29, 2005 12:08 PM

I didn't look particularly hard at the code, but, I should point out the wonderful Boost Libraries.

One of the Boost's libraries is Boost.SmartPointers, which is wonderful for this kind of thing. Example:

boost::shared_ptr< int > foo( new int );
boost::shared_ptr< int > bar( foo );

The integer pointed to by both foo and bar will only be destroyed once, automatically, and only when both go out of scope. A more complex example:

#include <boost/shared_ptr.hpp>using namespace boost;shared_ptr< int > global;int main ( void ) {    global = shared_ptr< int >( new int ); //global -> new int    shared_ptr< int > local = global; //global & local -> int    //show that we're sharing the same int:    *global = 3;    assert( *local == 3 );    global.reset(); //global -> nothing, local -> int    local.reset(); //global & local -> nothing, nothing -> int, int will be deleted now.    local = shared_ptr< int >( new int ); //local -> new int again}   //when local goes out of scope, it's automatically reset() so that you don't have a memory leak.

You can also pass your own special destroy function:

void destroy_and_free( int * foo ) {    //foo->~int(); //trivial dtor    free( foo );}int * malloc_and_create( void ) {    int * foo = (int *) std::malloc( sizeof( int ) );    new (foo) int( 3 ); //placement new 0_o.}int main ( void ) {    shared_ptr< int > foo( malloc_and_create() , destroy_and_free );    shared_ptr< int > bar( foo );    //the memory allocated will still have destroy_and_free called upon it, even though we didn't tell bar to, since it finds this out from foo.}

Definately some overlap so I just thought I'd point it out.

persil

199

Author

May 29, 2005 01:00 PM

Ok, thanks. I did indeed consider using Boost' smart pointers.

I guess boost::shared_ptr must used circular linked list internally to keep track of all pointers. I do the same with linked_pointer :) However, when I made it, I didn't consider using boost's. It works fine though. And it's something that's very easy to conceive, and fun too ;)

Anyway, why do I make a handle thingy? Consider the following example, and you'll see the difference between boost's shared_ptr (or my own linked_pointer) and my handle.

#include "Components.h"using namespace components;// Maybe you don't like the idea of a manager. I considered using a static// pool of handles, but then it would not be very flexible, and look-up times// could become a problem if specificity was not possible.handle_mgr manager;int main(){  {    int * ptr = new int;    // Create a new handle using pointer    handle< int > h1( manager, ptr );    // Create a copy of this handle    handle< int > h2 = h1;    // Create another handle of the same pointer    handle< int > h3( manager, ptr );  }  return 0;}

Now if you do this with boost' shared_ptr, will it work? With handle it will. If you tell me boost handle this, I'll drop my idea ;)

Why do I need this? Consider the following:

class ArchiveFile;class Archive{public:  handle< ArchiveFile > createfile( ... )  {    return handle< ArchiveFile >( g_manager, new ArchiveFile( this ) );  }};class ArchiveFile{private:  handle< Archive > m_parent_archive;public:  ArchiveFile( Archive & ref )    : m_parent_archive( g_manager, &ref )  {  }};

I don't know if this illustrates the kind of interaction between dynamic objects I am trying to establish. With this method, it will not be mandatory for the programmer to keep an explicit handle for the Archive parent class if he only wants to access one file within it, because the file will have its Archive handle, and the parent Archive will only be destroyed when the ArchiveFile will be too.

That's the only way I thought for objects to pass handles to themselves when creating dependant objects.

It greatly simplifies memory management too.

MaulingMonkey

1,729

May 29, 2005 02:18 PM

int * ptr = new int;handle< int > h1( manager, ptr );handle< int > h2 = h1;handle< int > h3( manager, ptr );

This is the first part that dosn't translate quite directly to boost::shared_ptr. With shared_ptr, one should only create smart_ptrs from other smart_ptrs, except for the one instance where the pointer is being created. E.g. you'd do this:

shared_ptr< int > ptr ( new int );shared_ptr< int > h1( ptr );shared_ptr< int > h2 = h1;shared_ptr< int > h3( ptr );

This is because the shared_ptrs don't share a manager class that they are forced to mantain. This means that doing this:

int * ptr = new int;shared_ptr< int > h1( ptr );shared_ptr< int > h2( ptr );

Can cause ptr to end up getting deleted twice (since shared_ptrs were constructed from the same raw poniter twice). So, this can be a problem if you're working with a lot of legacy raw pointers code - otherwise, just never store the raw pointer and you're good to go :-).

The second bit has to do with using the "this" pointer - it's a raw pointer. This is probably best dealt with by mantaining a boost::weak_ptr to oneself. Here's the version I come up with on my first try:

class ArchiveFile {    //self is not needed for this class, but is provided for consistancy:    boost::weak_ptr< Archive > self;    boost::shared_ptr< Archive > parent;public:    Archive( boost::shared_ptr< ArchiveFile > & initial_handle , boost::shared_ptr< Archive > parent )        : parent( parent )    {        initial_handle = boost::shared_ptr< Archive >( this );        self = initial_handle;    }};class Archive {    //this time we do need self, to pass to dependant ArchiveFile(s)    boost::weak_ptr< Archive > self;public:    Archive( boost::shared_ptr< Archive > & initial_handle ) {        initial_handle = boost::shared_ptr< Archive >( this );        self = initial_handle;    }    boost::shared_ptr< ArchiveFile > createfile( ... ) {        boost::shared_ptr< ArchiveFile > archive_file;        new ArchiveFile( archive_file , boost::shared_ptr< Archive >( self ) );        return archive_file;    }};class ArchiveManager { //creates/returns archives    //no self, since we gotta end the line somewhere...public:    boost::shared_ptr< Archive > createarchive( ... ) {        boost::shared_ptr< Archive > archive;        new Archive( archive );        return archive;    }};

Someone else can probably come up with a much less hacky way to do this.

Edit: Fixes, and related notes:

"new ArchiveFile( archive_file , boost::shared_ptr< Archive >( self ) );" - shared_ptr explicit cast necessary to upgrade from a weak_ptr - this will throw if weak_ptr ever points to a deleted item, but that should never happen (if self is deleted, then "this" has been deleted, and that means we should never be in a function referencing this or self in the first place).

"initial_handle = boost::shared_ptr< Archive >( this );" - this assumes that Archive is indeed being dynamically allocated. Note that we never take the return of new in these examples :-/.

Reason for this ugly "initial_handle = ..." stuff in the first place instead of just doing boost::shared_ptr< Archive > archive( new Archive( archive ) ); - danger will robson, Archive will be constructed with a non-constructed boost::shared_ptr, which is just plain bad mojo - weak_ptr will likely end up pointing to 0xBAADF00D or similar.

Finally, instead of this ugly hack, I'd instead define my classes so I can just do:

shared_ptr< ArchiveManager > manager ( new ArchiveManager );
shared_ptr< Archive > archive ( new Archive( manager , "name" );
shared_ptr< ArchiveFile > file( new ArchiveFile( archive , "filename" ) );

instead of:

shared_ptr< ArchiveManager > manager ( new ArchiveManager );
shared_ptr< Archive > archive( manager->createArchive( "name" );
shared_ptr< ArchiveFile > file( archive->createFile( "filename" ) );

This also has the benifit of not needing to write accessory functions createArchive and createFile.

[Edited by - MaulingMonkey on May 29, 2005 2:18:27 PM]

persil

199

Author

May 29, 2005 06:35 PM

Thanks, the idea is not bad :)

I hadn't thought about using a reference parameter in a constructor to return immediately the first handle. In some previous version of my lib, I enforced a static factory which always returned a handle, that would be fit for this case.

However, do you realise that, while doing this, I am more or less coding a handle manager for every class type. But thanks for the idea, I guess it would be less intrusive... Hmmm, I'll think about this.

snk_kid

1,312

May 29, 2005 06:40 PM

Quote:Original post by persil
I guess boost::shared_ptr must used circular linked list internally to keep track of all pointers.

No it uses non-intrusive reference counting.

On a different note but related to the thread, you most definitely wanna read this page and probably this too.

Weird STL container + void pointer behavior? [SOLVED]

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Weird STL container + void pointer behavior? [SOLVED]

This topic is closed to new replies.

Popular Topics

Recommended Tutorials

Reticulating splines