Author Topic: Fenton Webb's advice re "To dispose or not to dispose"  (Read 42268 times)

0 Members and 1 Guest are viewing this topic.

TheMaster

  • Guest
Fenton Webb's advice re "To dispose or not to dispose"
« on: August 02, 2012, 06:59:51 PM »
I think it's clear that the advice given on the adndevblog regarding this topic is not only misguided, but the one that offered it clearly didn't give much thought to its implications.

Code - C#: [Select]
  1.  
  2. using System;
  3. using System.Collections.Generic;
  4. using System.Linq;
  5. using System.Text;
  6. using Autodesk.AutoCAD.ApplicationServices;
  7. using Autodesk.AutoCAD.Runtime;
  8. using Autodesk.AutoCAD.EditorInput;
  9. using Autodesk.AutoCAD.DatabaseServices;
  10. using Autodesk.AutoCAD.Geometry;
  11.  
  12. namespace AutoCADProject1
  13. {
  14.    public static class Commands
  15.    {
  16.       // Straightforward add line to model space:
  17.       [CommandMethod( "AddLineToModel" )]
  18.       public static void AddLineToModel()
  19.       {
  20.          Document doc = Application.DocumentManager.MdiActiveDocument;
  21.          using( Transaction tr = doc.TransactionManager.StartTransaction() )
  22.          {
  23.             ObjectId msId = SymbolUtilityServices.GetBlockModelSpaceId( doc.Database );
  24.             BlockTableRecord model = (BlockTableRecord) msId.GetObject( OpenMode.ForWrite );
  25.             using( Line line = new Line( Point3d.Origin, new Point3d( 2.0, 2.0, 0.0 ) ) )
  26.             {
  27.                line.SetDatabaseDefaults( doc.Database );
  28.                model.AppendEntity( line );
  29.                tr.AddNewlyCreatedDBObject( line, true );
  30.                tr.Commit();
  31.             }
  32.          }
  33.       }
  34.  
  35.       /// Functionally the same as the above, but strictly following
  36.       /// the advice given by Fenton Webb (e.g., dispose *everything*) at:
  37.       /// http://adndevblog.typepad.com/autocad/2012/07/forcing-the-gc-to-run-on-the-main-thread.html:
  38.       ///
  39.       ///   "Therefore, I recommend you Dispose() all AutoCAD .NET objects using
  40.       ///   the .NET 'using' statement – that way you are not relying on the GC
  41.       ///   to clean up after yourself."
  42.  
  43.       [CommandMethod( "FentonsAddLineToModel")]
  44.       public static void FentonsAddLineToModel()
  45.       {
  46.          // Disposing the Document is not only unecessary, doing it will cause
  47.          // a failure on the next attempt to access that same Document:
  48.          using( Document doc = Application.DocumentManager.MdiActiveDocument )  
  49.          {
  50.             // Disposing the TransactionManager is completely unnecessary
  51.             using( Autodesk.AutoCAD.ApplicationServices.TransactionManager transman = doc.TransactionManager )
  52.             {
  53.                using( Transaction tr = transman.StartTransaction() )
  54.                {
  55.                   // Disposing the Database is also completely unnecessary:
  56.                   using( Database db = doc.Database )
  57.                   {
  58.                      ObjectId idModel = SymbolUtilityServices.GetBlockModelSpaceId( db );
  59.  
  60.                      // Disposing the BlockTableRecord is also completely unnecessary:
  61.                      using( BlockTableRecord model = (BlockTableRecord) idModel.GetObject( OpenMode.ForWrite ) )
  62.                      {
  63.                         using( Line line = new Line( Point3d.Origin, new Point3d( 2.0, 2.0, 0.0 ) ) )
  64.                         {
  65.                            line.SetDatabaseDefaults( db );
  66.                            model.AppendEntity( line );
  67.                            tr.AddNewlyCreatedDBObject( line, true );
  68.                            tr.Commit();
  69.                         }
  70.                      }
  71.                   }
  72.                }
  73.             }
  74.          }
  75.       }
  76.    }
  77. }
  78.  
« Last Edit: August 03, 2012, 12:45:00 AM by TonyT »

Keith™

  • Villiage Idiot
  • Seagull
  • Posts: 16827
  • Superior Stupidity at its best
Re: Bad Advice regarding "To dispose or not to dispose"
« Reply #1 on: August 02, 2012, 07:06:53 PM »
Seems kind of convoluted if you ask me
Proud provider of opinion and arrogance since November 22, 2003 at 09:35:31 am
CadJockey Militia Field Marshal

Find me on https://parler.com @kblackie

TheMaster

  • Guest
Re: Bad Advice regarding "To dispose or not to dispose"
« Reply #2 on: August 02, 2012, 07:36:04 PM »
Seems kind of convoluted if you ask me

What seems kind of convoluted?

Kerry

  • Mesozoic relic
  • Seagull
  • Posts: 11654
  • class keyThumper<T>:ILazy<T>
Re: Bad Advice regarding "To dispose or not to dispose"
« Reply #3 on: August 02, 2012, 08:27:40 PM »
Tony,
I've been waiting till I have a couple of days spare so I could seek clarification of that blog post.

The question I asked in the post that lead to the one you refer to wasn't answered to my satisfaction.

My argument is STILL that these concepts and requirements should be covered in the documentation ... but my requests have seemingly fallen on deaf ears.

The code composition demonstrated by your second example is exactly as I interpreted the blog post.

for reference, this was my original question
Quote
I have a question

If StartOpenCloseTransaction() if the preferred operation why have we NOT seen it in any AutoDesk samples in the last 7 years ??

and another ..

>>
Quote
Remember to always call Dispose on your AutoCAD.NET objects.
<<
would someone care to make a public definitive statement regarding the disposing of objects ?
... and is the process different when using
StartOpenCloseTransaction() ??

Regards
Kerry
Perfection is not optional.
Everything will work just as you expect it to, unless your expectations are incorrect.
Discipline: None at all.

--> Donate to theSwamp<--

Jeff H

  • Needs a day job
  • Posts: 6083
Re: Bad Advice regarding "To dispose or not to dispose"
« Reply #4 on: August 02, 2012, 09:08:53 PM »
Just to make sure I have an understanding is the call to AddNewlyCreatedDBObject() necessary if you already have the line wrapped in the using statement?
 
Does adding the the line to the transaction just call dispose for you or is there more going on?
 
 
 
Code - C#: [Select]
  1.  
  2.          [CommandMethod("AddLineToModel")]
  3.         public static void AddLineToModel()
  4.         {
  5.             Document doc = Application.DocumentManager.MdiActiveDocument;
  6.             using (Transaction tr = doc.TransactionManager.StartTransaction())
  7.             {
  8.                 ObjectId msId = SymbolUtilityServices.GetBlockModelSpaceId(doc.Database);
  9.                 BlockTableRecord model = (BlockTableRecord)msId.GetObject(OpenMode.ForWrite);
  10.                 using (Line line = new Line(Point3d.Origin, new Point3d(2.0, 2.0, 0.0)))
  11.                 {
  12.                     line.SetDatabaseDefaults(doc.Database);
  13.                     model.AppendEntity(line);
  14.                     //tr.AddNewlyCreatedDBObject(line, true);
  15.                     tr.Commit();
  16.                 }
  17.             }
  18.         }
  19.  

TheMaster

  • Guest
Re: Bad Advice regarding "To dispose or not to dispose"
« Reply #5 on: August 03, 2012, 12:26:51 AM »
Just to make sure I have an understanding is the call to AddNewlyCreatedDBObject() necessary if you already have the line wrapped in the using statement?


Yes, it's still required.  Disposing the Line ensures that if the code were to fail before the line was added to the database, the line is deterministically disposed, preventing its finalizer from running on the GC's thread, which could crash AutoCAD. You could probably get away without disposing the Line since there's little chance of a failure in that particular example, but in more complex situations where more code runs before the line is added to the database, it becomes more dangerous.

Quote

Does adding the the line to the transaction just call dispose for you or is there more going on?
 

Adding the Line to the transaction causes it to be treated the same as database-resident objects that were acquired from the transaction. When a transaction is committed, changes made to all objects in the transaction are committed (as if AcDbObject::close() was called on every open AcDbObject in the transaction). If a transaction is aborted, the changes made to every AcDbObject in the transaction are rolled back (as if AcDbObject::cancel() were called on each), so adding newly-created objects to a transaction (after adding them to a database) allows changes made to those objects to be handled in the same way.


TheMaster

  • Guest
Re: Bad Advice regarding "To dispose or not to dispose"
« Reply #6 on: August 03, 2012, 12:35:36 AM »

My argument is STILL that these concepts and requirements should be covered in the documentation ... but my requests have seemingly fallen on deaf ears.


Reading the documentation reveals numerous telltale signs suggesting that the writers were not all that experienced with the AutoCAD managed API, and of course, that would explain why there is so much missing from it.

Keith™

  • Villiage Idiot
  • Seagull
  • Posts: 16827
  • Superior Stupidity at its best
Re: Fenton Webb's advice re "To dispose or not to dispose"
« Reply #7 on: August 03, 2012, 01:34:15 AM »
Seems kind of convoluted if you ask me

What seems kind of convoluted?

Well, maybe I chose the wrong word .. I tend to do that.

Maybe I just don't see the bigger picture. Disposing of objects when you are done with them is kind of the point isn't it? I thought that the 'using' statement did just that, so why would another disposal be necessary .. unless the concept is that the garbage collector deals with items based upon how they are disposed.

My understanding was that as soon as objects went out of scope, .Net automatically manages the disposal of items not explicitly disposed ... and while it does rely on the garbage collector to do the dirty work, isn't that kind of the point with .Net?

I personally don't recommend leaving unfinished tasks to the GC, instead I like to dispose of items as soon as I am done with them (or as soon as practical). Doing it this way seems straightforward if you ask me ... but apparently others have different ideas ... or maybe that is just the C/C++ programmer speaking.
Proud provider of opinion and arrogance since November 22, 2003 at 09:35:31 am
CadJockey Militia Field Marshal

Find me on https://parler.com @kblackie

VVeli

  • Newt
  • Posts: 22
Re: Fenton Webb's advice re "To dispose or not to dispose"
« Reply #8 on: August 03, 2012, 03:07:14 AM »
Hi,
I have also worked with these Disposable objects but there is not a straight line what objects are MUST to dispose.
Have you read Kean´s article of that? That is the clearest article what I have seen.
http://through-the-interface.typepad.com/through_the_interface/2008/06/cleaning-up-aft.html
http://through-the-interface.typepad.com/through_the_interface/2008/06/cleaning-up-aft.html

Cheers
Veli V.

Kerry

  • Mesozoic relic
  • Seagull
  • Posts: 11654
  • class keyThumper<T>:ILazy<T>
Re: Fenton Webb's advice re "To dispose or not to dispose"
« Reply #9 on: August 03, 2012, 04:04:26 AM »

There is also this recent thread here.

http://www.theswamp.org/index.php?topic=40286.0;all

Hi Guys,

Stephen is correct. You dont have to use the "using" statement with existing DB resident objects that are opened with the transaction (provided the transaction object is created with the "using" statement).

I originally thought that we need to explicitly dispose off existing DB resident objects as well that are opened with a transaction but because the transaction will be disposed automatically in the UI thread in case of an exception (provided you use the "using" statement with the transaction object), the DB objects openeds with the transaction will also be disposed off properly.

To simplify:

1) Always use the "using" statement with the transaction object
2) Always use the "using" statement with newly created DB Objects being added to the transaction
3) Always using the "using" statement with DB Objects that are not added to the database
4) You dont have to use the "using" statement with existing DB Objects opened with a transaction object

Cheers
Gopinath
Perfection is not optional.
Everything will work just as you expect it to, unless your expectations are incorrect.
Discipline: None at all.

--> Donate to theSwamp<--

TheMaster

  • Guest
Re: Fenton Webb's advice re "To dispose or not to dispose"
« Reply #10 on: August 03, 2012, 12:52:09 PM »
Hi,
I have also worked with these Disposable objects but there is not a straight line what objects are MUST to dispose.
Have you read Kean´s article of that? That is the clearest article what I have seen.
http://through-the-interface.typepad.com/through_the_interface/2008/06/cleaning-up-aft.html
http://through-the-interface.typepad.com/through_the_interface/2008/06/cleaning-up-aft.html

Cheers
Veli V.

I've written about this many times, and agree that the rules aren't simple.

For DBObjects, it depends on two things:

If the DBObject is database-resident, and you got it from a Transaction, you don't have to dispose it.

If the DBObject is not database-resident, or is database-resident and you got it from ObjectId.Open(), then you must dispose it.

Objects whose Dispose() methods have side-effects must always be disposed. Those include the Transaction and DocumentLock. Both of those do things when when they are disposed. You can often tell if an object will perform some specific side-effect in their finalizer, by examining its AutoDelete property. When the value of this property is false, it usually means the object does nothing when its finalizer is called (either by the GC or by a call to Dispose()).

For example, when you get a Database from the Document's Database property, the AutoDelete property of the Database managed wrapper is false, because the drawing is open in the editor and for that reason its underlying AcDbDatabase must not be deleted. If on the other hand, you get a Database by calling the constructor (e.g., 'new Database(...)' ), and then you call ReadDwgFile(), the value of the AutoDelete property is true (and serves as the way of telling the Database managed wrapper that it must delete the wrapped AcDbDatabase).

For all DisposableWrappers, you must dispose them if code that runs from their Finalizer is not thread-safe, and could crash AutoCAD if it runs on the GC's thread. Unfortunately, that is not easy to determine, and perhaps is why Fenton gives the advice he does, but I still do not agree with it, or with the overall design which requires either guesswork, intimate knowledge of API internals, or taking no chances and grossly over-complicating client code by disposing *everything* as per my example above.

And here's something those Autodesk people didn't tell you:

Avoiding calls to Dispose() when it is unnecessary can actually improve the performance of AutoCAD and your plug-ins. The reason for that is because by default, the GC runs on a high-priority background thread, which means that on a multiple-processor system (do you know anyone that still runs AutoCAD on a single-processor system?), the GC can execute on a different (and presumably under-utilized) processor, concurrently, while AutoCAD and your plug-in run on another processor (e.g., a form of parallel execution). And, since AutoCAD doesn't leverage multiple processors, work done by the GC can be offloaded to another CPU core with virtually no performance impact on AutoCAD and your plug-in.

So, if you do follow the advice of Fenton and Stephen, the work that must be done by Dispose() will be done synchronously, requiring AutoCAD and your plug-in to wait for it to complete, which is not the case if you just allow the GC to take care of it on another under-utilized CPU core.

Oh and BTW, does that also give a hint as to why disabling concurrent garbage collection in AutoCAD is a really bad idea? If you do choose to do that, you should be the one sitting at that PC waiting the extra time and experiencing the lag in the UI during interactive operations like dragging. If you are going to do that for someone else only because it eliminates the crashes from the bugs in your code, do it only as a temporary stopgap measure, fix the bugs as soon as possible and re-enable concurrent garbage collection, as it should be.
« Last Edit: August 03, 2012, 03:35:24 PM by TT »

TheMaster

  • Guest
Re: Fenton Webb's advice re "To dispose or not to dispose"
« Reply #11 on: August 03, 2012, 01:55:21 PM »
Seems kind of convoluted if you ask me

What seems kind of convoluted?

Well, maybe I chose the wrong word .. I tend to do that.

Maybe I just don't see the bigger picture. Disposing of objects when you are done with them is kind of the point isn't it? I thought that the 'using' statement did just that, so why would another disposal be necessary .. unless the concept is that the garbage collector deals with items based upon how they are disposed.

My understanding was that as soon as objects went out of scope, .Net automatically manages the disposal of items not explicitly disposed ... and while it does rely on the garbage collector to do the dirty work, isn't that kind of the point with .Net?

I personally don't recommend leaving unfinished tasks to the GC, instead I like to dispose of items as soon as I am done with them (or as soon as practical). Doing it this way seems straightforward if you ask me ... but apparently others have different ideas ... or maybe that is just the C/C++ programmer speaking.

The whole point to writing code that runs in a 'managed' execution environment, is so that the programmer can rely on the GC to clean up after themself, and offload tasks that C/C++ programmers must routinely do manually (e.g, free memory and call d'tors) or automate with the help of smart pointers.

When we write managed code, we are always relying on the GC to clean up the managed objects we create and use (whether they implement IDisposable or not), and we do not need to be concerned with deallocating those objects or the memory they consume, calling d'tors, etc.

The purpose of IDisposable is to provide a means for the user of class to deterministically tell an instance of the class that they are done using it, and what an object does in response to that signal is entirely up to the designer of the class. For many objects, IDisposable is a clean way to perform operations that must always be performed after code has finished using an object, but should not be delayed until the GC collects the object. For example, if an object represents a file on disk, or a registry key, it can implement IDisposable and in its implementation of Dispose() it can close the open file or registry key. 

The reason for all of the confusion in the case of AutoCAD's managed API, is that many AutoCAD managed types implement IDisposable even when it is unnecessary, and largely out of convenience. Many managed types derive from DisposableWrapper (which is the class that implements IDisposable), and in some cases, do so because they must also derive from the RXObject base type (which is derived from DisposableWrapper), because the managed class represents a native object that is an instance of AcRxObject.

So, what we have in the case of AutoCAD's managed API are many types that implement IDisposable largely out of coincidence and/or convenience, rather than because they must do something meaningful in their implementation of Dispose().
« Last Edit: August 03, 2012, 02:14:36 PM by TT »

Keith™

  • Villiage Idiot
  • Seagull
  • Posts: 16827
  • Superior Stupidity at its best
Re: Fenton Webb's advice re "To dispose or not to dispose"
« Reply #12 on: August 03, 2012, 03:20:16 PM »
From my perspective that kinda proves my point .. that being that .Net manages those objects, many times better than the person writing the code would.

Is it possible that Autodesk's managed API is merely a bandaid version? After all, alot of time folks do things just because that is the way they have always done them. Why should Autodesk be any different?
Proud provider of opinion and arrogance since November 22, 2003 at 09:35:31 am
CadJockey Militia Field Marshal

Find me on https://parler.com @kblackie

TheMaster

  • Guest
Re: Fenton Webb's advice re "To dispose or not to dispose"
« Reply #13 on: August 03, 2012, 05:41:28 PM »
From my perspective that kinda proves my point .. that being that .Net manages those objects, many times better than the person writing the code would.

Is it possible that Autodesk's managed API is merely a bandaid version? After all, alot of time folks do things just because that is the way they have always done them. Why should Autodesk be any different?

I don't think it can be called a band-aid, and the architect (Albert Szilvasy) deserves a lot of credit for what is for the most part, a well-designed managed API layer, but I'd also dare to speculate that given what everyone, including Albert has learned about the API and the functional requirements and usage patterns to date, it might have been done somewhat differently if it was completely redone from scratch.

dgorsman

  • Water Moccasin
  • Posts: 2431
Re: Fenton Webb's advice re "To dispose or not to dispose"
« Reply #14 on: August 03, 2012, 05:45:27 PM »
Do you think that total re-write could be done without significantly breaking existing code?  And in a timeframe consistent with the rate at which tools, .NET frameworks, etc. are released and retired?
If you are going to fly by the seat of your pants, expect friction burns.

try {GreatPower;}
   catch (notResponsible)
      {NextTime(PlanAhead);}
   finally
      {MasterBasics;}