Disposing issue
in [CSharp]
|
From: Peter Duniho on 3 Feb 2010 18:01 JohnS wrote: > 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"). Yes, I know. I acknowledged that in my follow-up reply to my own post. That's not really a problem at all though. > This is hardly elegant to say the least and certainly > error-prone to expect undisciplined programmers to constantly do this > (just the reality). As I pointed out, there are multiple ways to approach the problem. Passing the DataTable back is not necessarily the best way to solve it in your case, but that doesn't mean it's never going to be the best way to solve it. > 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. First: if you would find yourself cloning the _table_, then it actually makes more sense to pass the table back in the first place. If you're only cloning a row of data for a specific purpose, that is trivial overhead and not a problem at all. Second: note that the cloning does not literally have to produce the same type (though in most cases, even for Designer-generated types, this shouldn't be much of a problem). The point is to extract the data you need and return that in some useful way. > 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.). Frankly, IMHO creating a whole table when just one row would do is ugly. How can you worry about the overhead involved in copying some data from a container to a more stable format you can use, when your entire implementation is using a much higher overhead approach than is required? > 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. Perhaps it's because I don't use the database classes much, but it's not clear to me why, if DataRow itself isn't disposable, having the parent DataTable object disposed is a problem. Just because an object implements IDisposable, that doesn't mean that every piece of information you might get from it becomes invalidated when the object itself is disposed. And this is doubly true if you really believe that the DataTable.Dispose() method doesn't do anything useful. How could it cause a problem at all to return a DataRow from a DataTable that has been disposed, if disposing the DataTable itself does nothing? > 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. Note that if you're keeping a reference to a DataRow obtained from the DataTable, between the two options "dispose the DataTable" and "don't dispose the DataTable", the current behavior appears to be mostly the same either way, with some specific exceptions: � A class that implements IDisposable also has a finalizer. Objects with finalizers cannot be collected as quickly as those without, so even ignoring anything else, failing to dispose such objects will make your code run more poorly. � A class that implements IDisposable where disposing today doesn't release unmanaged resources is not necessarily going to have that characteristic in the future. If and when that happens, failing to release unmanaged resources will cause even more significant problems to the execution of the code. � If you do choose to dispose the DataTable, but keep the reference to the DataRow, then there may never be any problem with that. There's nothing obvious about a DataRow that suggests the parent DataTable has to remain undisposed for the DataRow instance itself to remain useful. � Even if in the future, using the DataRow when the DataTable has been disposed is a problem, the manifestation of that problem will be immediate and easily observed. On the other hand, failure to manage memory correctly can and will manifest itself in much more subtle and harder-to-track ways. By the time that failure is an actual problem, you may not even be the one maintaining the code, but even if you are, the chances that you will immediately realize the problem is tied to the failure to dispose an object in a specific place in the code is practically nil. My personal opinion is that there's a much more correct approach to this problem, and that involves either moving management of the lifetime of the DataTable to the consuming code, or simply not even depending on the DataTable outside the method where you dispose it, by copying the data of interest and returning that. Even better would be to not design the code so that you go to all the trouble of creating a whole DataTable instance just to get one row's worth of data. But, if you insist on doing something incorrect, IMHO the much better incorrect choice to make is to dispose the DataTable and return the DataRow anyway. This will be much more maintainable going forward, as it's more efficient even today (suppressing finalizing of the object) and if the object should in fact require disposal in the future, any failure that might occur will be easily fixed, rather than lurking as a hard-to-discover program efficiency and resource management problem. Pete
From: Mr. Arnold on 3 Feb 2010 18:41 Tom Shelton wrote: > 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? > Yes, it has been short circuited with the return like that. I consider that to be a bad programming practice. On top of that, if the application using a Using statement aborts in the middle of a database connection that is doing something after the connection open, it's not disposing or closing anything, because it never made it to the Dispose in the Using statement. I am speaking from first hand experience on this where I wrote a multi threaded console application that ran 24/7*365 consuming a 3rd party Web service getting XML and parsing it out to various SQL server tables at the client's site. The SQL server table was changed while this application was running, the application blew on the thread of the Using statement after the connect was opened down in code that was working with fields. It blew, short circuited the Using statement and left, not closing or disposing of anything. It ran all night in that condition until there were no more SQL connections to be had by anything else. I corrected the problem by doing a try/catch/finally (with implicit close of the connection) no matter what. Oh, I know the nightmare of Using statement. I am on a project to speed up a Web site, the users say it's too slow. One of the problems is SQL server is being hammered with a continuous spikes of 2,400 connections being used throughout the day. And guess what? The DAL is loaded with Using statements connected to the database, and I know that Using statement is not closing connections, and Garbage Collection must come behind everything disposing of objects releasing resources on SQL server. There are other problems with Web site speed.
From: Peter Duniho on 3 Feb 2010 19:00 Mr. Arnold wrote: > Yes, it has been short circuited with the return like that. I consider > that to be a bad programming practice. On top of that, if the > application using a Using statement aborts in the middle of a database > connection that is doing something after the connection open, it's not > disposing or closing anything, because it never made it to the Dispose > in the Using statement. Please stop posting false information about the "using" statement. The entire point of the "using" statement is that it is not possible to "short-circuit" it, or otherwise avoid disposal of the object. The fact that it's somewhat more convenient is simply a happy side-effect. The real point of the "using" statement is that it ensures that no matter how you exit the block of code the "using" statement protects, the target object for the "using" statement WILL BE DISPOSED. It does this by creating a try/finally block on your behalf. > [...] > The SQL server table was changed while this application was running, the > application blew on the thread of the Using statement after the connect > was opened down in code that was working with fields. It blew, short > circuited the Using statement and left, not closing or disposing of > anything. You'll have to be more specific about your use of the word "blew". It's not a standardized technical term. It is true that if the whole thread is simply stopped, the dispose can't happen. But, there is no way to accomplish that in managed code. Even a call to Thread.Abort() will throw an exception, which will propagate back through any "using" statements in the call stack. If you forcefully take down the thread using unmanaged code, or the entire process, then sure�you won't get a call to Dispose(). But the same is true if you pull the plug on the computer. These are not scenarios that have any relevance in this discussion. > It ran all night in that condition until there were no more > SQL connections to be had by anything else. I corrected the problem by > doing a try/catch/finally (with implicit close of the connection) no > matter what. If doing that fixed the code, then you didn't have the correct object in the "using" statement in the first place. And you could have fixed the problem simply by putting the current object in, rather than duplicating exactly what a "using" statement already does. > Oh, I know the nightmare of Using statement. I am on a project to speed > up a Web site, the users say it's too slow. One of the problems is SQL > server is being hammered with a continuous spikes of 2,400 connections > being used throughout the day. And guess what? The DAL is loaded with > Using statements connected to the database, and I know that Using > statement is not closing connections, and Garbage Collection must come > behind everything disposing of objects releasing resources on SQL > server. There are other problems with Web site speed. Utter baloney. Whatever problem you're seeing, it has nothing at all to do with the failure of an object the target of a "using" statement failing to be disposed of when the "using" block is exited. Show me code that has a reproducible problem when you have a "using" statement, but not when you replace that "using" statement with a try/finally block, and I will point out to you why your implementation with the try/finally block isn't using the same object the implementation with the "using" statement was. Pete
From: Mr. Arnold on 3 Feb 2010 19:40 Peter Duniho wrote: I don't know where the other post went, but here is my reply Peter Duniho wrote: > Mr. Arnold wrote: >> Yes, it has been short circuited with the return like that. I consider that to be a bad programming practice. On top of that, if the application using a Using statement aborts in the middle of a database connection that is doing something after the connection open, it's not disposing or closing anything, because it never made it to the Dispose in the Using statement. > > Please stop posting false information about the "using" statement. > > The entire point of the "using" statement is that it is not possible to "short-circuit" it, or otherwise avoid disposal of the object. And you're wrong man, as I have had first hand experience with the nightmare of the Using statement. > > The fact that it's somewhat more convenient is simply a happy side-effect. The real point of the "using" statement is that it ensures that no matter how you exit the block of code the "using" statement protects, the target object for the "using" statement WILL BE DISPOSED. > > It does this by creating a try/finally block on your behalf. No it doesn't and you can go out to Google and search it out. That's what it's suppose to do, but this is MS. > >> [...] >> The SQL server table was changed while this application was running, the application blew on the thread of the Using statement after the connect was opened down in code that was working with fields. It blew, short circuited the Using statement and left, not closing or disposing of anything. > > You'll have to be more specific about your use of the word "blew". It's not a standardized technical term. Blew-up, terminated, aborted.... > > It is true that if the whole thread is simply stopped, the dispose can't happen. But, there is no way to accomplish that in managed code. Even a call to Thread.Abort() will throw an exception, which will propagate back through any "using" statements in the call stack. I don't know. It blew up on the main thread and the application kept running went back to sleep and woke up again. > > If you forcefully take down the thread using unmanaged code, or the entire process, then sure�you won't get a call to Dispose(). But the same is true if you pull the plug on the computer. These are not scenarios that have any relevance in this discussion. > >> It ran all night in that condition until there were no more SQL connections to be had by anything else. I corrected the problem by doing a try/catch/finally (with implicit close of the connection) no matter what. > > If doing that fixed the code, then you didn't have the correct object in the "using" statement in the first place. And you could have fixed the problem simply by putting the current object in, rather than duplicating exactly what a "using" statement already does. So you say, but again this is MS. I never had the problem again since the fix to the SQL server table corrected the problem. > >> Oh, I know the nightmare of Using statement. I am on a project to speed up a Web site, the users say it's too slow. One of the problems is SQL server is being hammered with a continuous spikes of 2,400 connections being used throughout the day. And guess what? The DAL is loaded with Using statements connected to the database, and I know that Using statement is not closing connections, and Garbage Collection must come behind everything disposing of objects releasing resources on SQL server. There are other problems with Web site speed. > > Utter baloney. Whatever problem you're seeing, it has nothing at all to do with the failure of an object the target of a "using" statement failing to be disposed of when the "using" block is exited. I don't see you standing in my shoes in conversations with other senior ..NET programmers about what is happening. > > Show me code that has a reproducible problem when you have a "using" statement, but not when you replace that "using" statement with a try/finally block, and I will point out to you why your implementation with the try/finally block isn't using the same object the implementation with the "using" statement was. > I can't show you any code, because I would have to shoot you, a DoD kind of thing. Do you understand? On top of that, MS 'Best Practice for Web Performance' says otherwise, and that implicit try/catch/finally is a best practice.
From: Mr. Arnold on 3 Feb 2010 21:16
Peter Duniho wrote: <snipped> What I see here Peter, if someone says anything that disagrees with you, or is somehow detrimental to MS on an area, then the moderators will block reading of the post. That's real good man real good. It's damage control I guess, as seen by the same tactics being used in the MS Vista forums. You may see the Using statement with rose colored glasses, I don't see it in that light, I know it has problems and it doesn't work as advertised 100% of the time. Yeah, MS and its problems with inferior products at times. It will never face it. |