Disposing issue
in [CSharp]
|
From: Tom Shelton on 3 Feb 2010 16:41 On 2010-02-03, Mr. Arnold <Arnold(a)Arnold.com> wrote: > JohnS wrote: >> Hi there, >> >> I'm currently using the following pattern in various place but just >> realized it's probably not safe. Can someone comment on this: >> >> public DataRow GetWhatever() >> { >> using (DataTable dataTable = GetDataTable()) >> { >> return dataTable[0]; >> } >> } >> >> The problem is that "DataRow" has a "Table" property that points back to >> the same "dataTable" in the "using" statement above. That object is >> disposed of in the "using" statement however so I assume the "Table" >> property will now point to garbage (or rather an object that shouldn't >> be used anymore). Is this correct? Note BTW that the indexer above is >> actually present in my case (the "DataTable" is actually a "DataTable" >> derivative created using the VS dataset designer) > > The real problem is you did a return in the middle of the using > statement, so the using statement has been short circuited. > > The return should be after the using statement. > > It should be like this. > > public DataRow GetWhatever() > { > var dr = new DataRow(); > > using (DataTable dataTable = GetDataTable()) > { > dr = dataTable[0]; > } > > return dr; > } Ummmm... No. it will not short circuit the using - the object dataTable will always be disposed.... Simple example: using System; namespace ConsoleApplication66 { class Program { static void Main ( string[] args ) { SomeMethod (); } static void SomeMethod () { Console.WriteLine ( "Entering Using" ); using ( ADisposableObject a = new ADisposableObject () ) { Console.WriteLine ( "Returning..." ); return; } } } class ADisposableObject : IDisposable { public void Dispose () { Console.WriteLine ( "Bye! Bye!" ); } } } output: Entering Using Returning... Bye! Bye! Maybe you meant something different? -- Tom Shelton
From: Peter Duniho on 3 Feb 2010 16:41 Mr. Arnold wrote: > The real problem is you did a return in the middle of the using > statement, so the using statement has been short circuited. Completely false. > The return should be after the using statement. There's no difference between having the return statement in the "using" or outside. > It should be like this. > > public DataRow GetWhatever() > { > var dr = new DataRow(); > > using (DataTable dataTable = GetDataTable()) > { > dr = dataTable[0]; > } > > return dr; > } That does exactly what the original code does. The object is still disposed, and if it was invalided by having its parent object (the DataTable) disposed in the original example, then it is still invalidated by having its parent object disposed in your example. Pete
From: Peter Duniho on 3 Feb 2010 16:49 Peter Duniho wrote: > [...] > As for the specific example, again� the correct solution depends on the > lifetime of the object. If the lifetime of the object need not extend > beyond the lifetime of the caller, then the caller can simply use a > "using" statement to manage that: > > public void DoWhicever() > { > using (DataRow row = GetWhatever()) > { > // do stuff > } > } Sorry, I neglected to make the code example address the specific example you actually had. That was silly. The original disposable object in your example is of course the entire DataTable. So, you have two options: delegate management of the lifetime of the DataTable object to the caller, and let it deal with extracting a specific row; or, clone the data from the DataTable in some useful way, and pass the clone back so that the DataTable itself can be disposed of in the called method. Which is the best approach depends entirely on the rest of the design. For example, where the DataTable instance came from and how the underlying data structures related to it are being managed. But in general, the latter approach will involve the least modification to the overall code base, of course. Pete
From: JohnS on 3 Feb 2010 17:34 > Sorry, I neglected to make the code example address the specific example > you actually had. That was silly. > > The original disposable object in your example is of course the entire > DataTable. So, you have two options: delegate management of the lifetime > of the DataTable object to the caller, and let it deal with extracting a > specific row; or, clone the data from the DataTable in some useful way, > and pass the clone back so that the DataTable itself can be disposed of in > the called method. > > Which is the best approach depends entirely on the rest of the design. For > example, where the DataTable instance came from and how the underlying > data structures related to it are being managed. But in general, the > latter approach will involve the least modification to the overall code > base, of course. Thanks again for your insight. Unfortunately it just doesn't work out very well in this case. First, "DataRow" doesn't inherit from "IDisposable" which puts the onus on the user to explicitly call "DataRow.Table.Dispose()" (instead of putting a "using" statement around "DataRow"). This is hardly elegant to say the least and certainly error-prone to expect undisciplined programmers to constantly do this (just the reality). Cloning or copying tables of data is also grossly inefficient for a DB application in particular and probably even more problematic in this case, since I would have to clone types that are created by the Visual Studio forms designer itself. Not sure if this is doable at all off-hand - would have too look into it but it almost certainly means more overhead when less is always better. Returning the "DataTable" object itself is also syntactically ugly when the table contains just one row in these cases (used for key searches, etc.). It's always the first and only row in the table users are after so returning the row is much cleaner. I'm resigned at this point to not worry about disposing of these "DataTable" objects for now since AFAIK, they consume no unmanaged resources I probably have to worry about. That is, they pick up "IDisposable" further up the hierarchy and various discussions about it on the web seem to confirm this. It's ultimately a trade-off however between pedantic adherence to proper "IDisposable" rules (which I'd prefer to follow), vs the trouble it's causing in this case.
From: Mr. Arnold on 3 Feb 2010 17:55
JohnS wrote: > Cloning or copying tables of data is also grossly > inefficient for a DB application in particular and probably even more > problematic in this case, since I would have to clone types that are > created by the Visual Studio forms designer itself. A good database application should have an abstraction layer away from the database, which makes for a more scalable and workable solution. The problem is the usage of outdated technology -- datatables. The second thing is not disconnecting from the data source via another object. |