Author Topic: AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)  (Read 8297 times)

0 Members and 1 Guest are viewing this topic.

pkohut

  • Guest
AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)
« on: February 21, 2010, 10:00:17 AM »
This post started out being a RFC, but as I was writing it up I pretty much answered my own question and now it's more or less a tutorial. Still, think of it as a RFC, just without the question as you'll probably be able to infer the general gist of the now non-question.  :roll:

When acquiring a new object pointer from most ARX api's the object is passed by reference in the function call, and the function's return is usually Acad::ErrorStatus. Being what it is, this doesn't lend itself to using smart pointers very cleanly, and you end up with 2 objects, 1 for the pointer and another for the smart pointer.

Listing 1 is how I normally handle this, as you see there are 2 objects dealing with the same iterator. I have to be sure to NULL out the original pointer once it is handed off to the smart pointer, otherwise I risk using it after the smart pointer is destroyed.

So the deal is I want this handled automatically and after about an hour of futzing around with different methods I settled on listing 2 and 2a.  Bottom line is it's simple, and with the name AssignToSmartPointer I'll be sure to use it only with some sort of smart pointer.

If for some reason the smart pointer constructor fails it is going to throw std::bad_alloc, so as long as the original pointer is good and std::bad_alloc isn't thrown, then the smart pointer can safely be dereferenced.

In the isolated confines of the examples below, this is pretty simple stuff. However, when working inside of an if statement 3 or 4 block levels deep, every little bit of help helps.

Listing 1
Code: [Select]
   AcDbLayerTableIterator * pIterator;
   es = pLayerTable->newIterator(pIterator);
   if(es !=  Acad::eOk)
      return es;

[color=navy]   boost::scoped_ptr<AcDbLayerTableIterator> pIter(pIterator);   // take control of the pointer and set the
   pIterator = NULL;                                             // set the original pointer to 0
[/color]

Listing 2
Code: [Select]
   AcDbLayerTableIterator * pIterator;
   es = pLayerTable->newIterator(pIterator);
   if(es !=  Acad::eOk)
      return es;
[color=navy]
   boost::scoped_ptr<AcDbLayerTableIterator> pIter(AssignToSmartPointer(pIterator));
[/color]

Listing 2a (template definition for AssignToSmartPointer)
Code: [Select]
template<typename T>
inline T * AssignToSmartPointer(T *& pOriginalPointer)
{
   T * pPtr = pOriginalPointer;
   pOriginalPointer = NULL;
   return pPtr;
}

owenwengerd

  • Bull Frog
  • Posts: 441
Re: AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)
« Reply #1 on: February 21, 2010, 10:32:21 AM »
I'm not familiar with boost, but any smart pointer that takes ownership of a pointer should have a constructor that accepts a reference to a non-const pointer, then sets the original pointer to NULL after ownership is transferred successfully.

LE3

  • Guest
Re: AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)
« Reply #2 on: February 21, 2010, 10:40:42 AM »
I always have followed the rule of opening for read the table, then after the newIterator it is created, to close the table, do the scan, and at the end delete the iterator, I use smart pointers a lot, but never for these cases.

edit: spelling
« Last Edit: February 21, 2010, 12:10:56 PM by LE3 »

pkohut

  • Guest
Re: AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)
« Reply #3 on: February 21, 2010, 03:17:04 PM »
I'm not familiar with boost, but any smart pointer that takes ownership of a pointer should have a constructor that accepts a reference to a non-const pointer, then sets the original pointer to NULL after ownership is transferred successfully.

Good point and I'll investigate further when I get back. Alas, from what I know the shared_ptr template and family do not do this.

pkohut

  • Guest
Re: AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)
« Reply #4 on: February 21, 2010, 03:46:58 PM »
I always have followed the rule of opening for read the table, then after the newIterator it is created, to close the table, do the scan, and at the end delete the iterator

Absolutely!

What prompted this is that the function that creates the iterator calls another function that has the potential to throw. So all the aquired pointer resources are in their own smart pointers. If the throwing function does throw I want the resouces handled, ie. deleted or closed in the case for opened entites (handled with AcDbObjectPointerBase derived classes).  This eliminates the need for try...catch blocks.


It's Alive!

  • BricsCAD
  • Needs a day job
  • Posts: 7033
  • AKA Daniel
Re: AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)
« Reply #5 on: February 21, 2010, 05:33:56 PM »
I always have followed the rule of opening for read the table, then after the newIterator it is created, to close the table, do the scan, and at the end delete the iterator

Absolutely!

...



That's not right though, well at least according to the docs.

Quote
When you create a new iterator, you are also responsible for deleting it. A symbol table should not be closed until all of the iterators it has constructed have been deleted

It's Alive!

  • BricsCAD
  • Needs a day job
  • Posts: 7033
  • AKA Daniel
Re: AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)
« Reply #6 on: February 21, 2010, 05:42:10 PM »

how about this? (it works)  :laugh:
Code: [Select]
boost::scoped_ptr<AcDbBlockTableRecordIterator> pIter;
if(pCurBtr->newIterator((AcDbBlockTableRecordIterator*&)pIter) != Acad::eOk)
  ...
« Last Edit: February 21, 2010, 05:50:43 PM by Daniel »

owenwengerd

  • Bull Frog
  • Posts: 441
Re: AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)
« Reply #7 on: February 21, 2010, 07:02:25 PM »
Daniel, I thought about mentioning that one could just as well add an operator T*&() to the smart pointer class so that the smart pointer could be passed as the argument to newIterator, but this exposes a potential weakness. If the newIterator() function does not return Acad::eOk, then the pointer is invalid and should not be deleted. That means you would have to add code to explicitly release ownership of the pointer without deleting it when the return value is not Acad::eOk (or throw caution to the wind and assume it will always be NULL when the function returns an error).

It's Alive!

  • BricsCAD
  • Needs a day job
  • Posts: 7033
  • AKA Daniel
Re: AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)
« Reply #8 on: February 21, 2010, 07:53:09 PM »
... (or throw caution to the wind and assume it will always be NULL when the function returns an error).

Ah, good point! I was assuming(I'm probably wrong) that the pointer would always be NULL, otherwise how would the newIterator() function know what status to return? I.e. either Autodesk has overridden new to return NULL on failure or a std::bad_alloc  is thrown .?.?.

pkohut

  • Guest
Re: AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)
« Reply #9 on: February 21, 2010, 08:50:17 PM »
That's not right though, well at least according to the docs.

Thanks for pointing that out as I've got a couple programs to go fix, and probably read that a long time ago and forgot (didn't pay attention).  Prior to 2005 or so this would not have been an issue cause I practiced single entry single exit programming, but at some point adjusted to having multiple exit points and closing entities as Louis described so there wouldn't be 6+ levels of nested if blocks.

owenwengerd

  • Bull Frog
  • Posts: 441
Re: AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)
« Reply #10 on: February 21, 2010, 09:09:40 PM »
The docs are wrong; it is not necessary to hold the collection open while iterating over it. There may be cases where it's a good idea (i.e. when you need to prevent changes until iteration is complete), and I suspect that's where the statement originates.

LE3

  • Guest
Re: AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)
« Reply #11 on: February 21, 2010, 09:10:16 PM »

That's not right though, well at least according to the docs.

Quote
When you create a new iterator, you are also responsible for deleting it. A symbol table should not be closed until all of the iterators it has constructed have been deleted


Thank you Dan.

Are you are reading this from an old arxref doc, no?

This is about AcDbLayerTable on the arxref for 2010
Quote
This function creates an object that can be used to iterate through the contents of the table and sets pIterator to point to it. If atBeginning is true the iterator begins its scan at the beginning of the table; otherwise it begins at the end of the table. If the argument skipDeleted is true, the iterator is initially positioned at the first or last non deleted record. Otherwise it is initially positioned at the first or last record, regardless of whether or not it has been deleted.

Each iterator returned from this function counts as a reader of the symbol table. As such, these iterators may only be constructed when the table is open for read. In addition, the table is not considered closed unless all of the iterators it has constructed have been deleted.
 

Never had any issues, on all my previous use of closing the table :) - and yes found some code where I did the close of the table after the delete of the iterator.

What I understood way back then, is that once you open your table for read, and have your pointer to your iterator, the table can be closed, is not needed anymore.

pkohut

  • Guest
Re: AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)
« Reply #12 on: February 21, 2010, 09:14:59 PM »
Thank you Dan.

Are you are reading this from an old arxref doc, no?

This is about AcDbLayerTable on the arxref for 2010

Look in the ARX Developer Guide "Iterators".

pkohut

  • Guest
Re: AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)
« Reply #13 on: February 21, 2010, 09:29:20 PM »
Modififed code below to clarify intent.
... (or throw caution to the wind and assume it will always be NULL when the function returns an error).

Ah, good point! I was assuming(I'm probably wrong) that the pointer would always be NULL, otherwise how would the newIterator() function know what status to return? I.e. either Autodesk has overridden new to return NULL on failure or a std::bad_alloc  is thrown .?.?.

In something like this
Code: [Select]
AcDbLayerTableIterator * pIter[color=red] = 0;[/color]
es = pTable-newIterator(pIter);

there is no guarantee what pIter will be if es is not Acad::eOk, so it's best not to program against an observed behavior.  When changes are made in the future and/or compiler optimization change the observed behaviors, these bugs can be a nightmare to track down.
« Last Edit: February 21, 2010, 10:28:24 PM by pkohut »

LE3

  • Guest
Re: AcDbSymbolTableIterator and shared_ptr (scoped_ptr as well)
« Reply #14 on: February 21, 2010, 09:31:12 PM »
Thank you Dan.

Are you are reading this from an old arxref doc, no?

This is about AcDbLayerTable on the arxref for 2010

Look in the ARX Developer Guide "Iterators".

Thank you Paul.

Yes, it is also on old arxref's docs that statement, it even have a link to a code sample.

I learn to do my own - don't trust much on the sample code - provided in there and prefer to do my own trials an errors. :)