Prev: TDD - Supplying a set of rows to a single Test multiple times
Next: WIN32 DLL with C#
From: JohnS on 3 Feb 2010 14:24
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)

From: Peter Duniho on 3 Feb 2010 15:12
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)

Yes, you definitely don't want to dispose objects that are being
returned for use by the caller.

The "using" statement is great for situations where you create an object
that is to be used locally, and for which its lifetime should not extend
beyond the method call in which the "using" statement is found. But
it's not a panacea. If your disposable object needs to live longer than
the method in which it's been created, you need to manage the lifetime
of the object more explicitly, by setting up some guidelines with
respect to ownership of the object and ensuring that the code that winds
up with ownership of the object does the necessary call to Dispose()
when it's actually done with it.

Pete
From: Mr. Arnold on 3 Feb 2010 16:01
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;
}
From: JohnS on 3 Feb 2010 16:15
> Yes, you definitely don't want to dispose objects that are being returned
> for use by the caller.
>
> The "using" statement is great for situations where you create an object
> that is to be used locally, and for which its lifetime should not extend
> beyond the method call in which the "using" statement is found. But it's
> not a panacea. If your disposable object needs to live longer than the
> method in which it's been created, you need to manage the lifetime of the
> object more explicitly, by setting up some guidelines with respect to
> ownership of the object and ensuring that the code that winds up with
> ownership of the object does the necessary call to Dispose() when it's
> actually done with it.

Thanks for the feedback. I think that (unmanaged) resource management in
..NET is a potential trip wire in cases like this. It can certainly be
difficult to manage anyway. I simply want to return a "DataRow" in this case
but now what. Clients should explicitly call "dataRow.Table.Dispose()"? From
what I understand, you don't even need to worry about disposing ADO.NET
objects like "DataTable" in current releases but it inherits from
"IDisposable" so in theory it should be disposed of (AFAIK). I'm not going
to worry about it however. I'm going to remove those "using" statements and
let the GC clean things up (for ADO.NET objects anyway). Thanks again for
your help.

From: Peter Duniho on 3 Feb 2010 16:39
JohnS wrote:
> Thanks for the feedback. I think that (unmanaged) resource management in
> ..NET is a potential trip wire in cases like this. It can certainly be
> difficult to manage anyway. I simply want to return a "DataRow" in this
> case but now what. Clients should explicitly call
> "dataRow.Table.Dispose()"? From what I understand, you don't even need
> to worry about disposing ADO.NET objects like "DataTable" in current
> releases but it inherits from "IDisposable" so in theory it should be
> disposed of (AFAIK). I'm not going to worry about it however. I'm going
> to remove those "using" statements and let the GC clean things up (for
> ADO.NET objects anyway). Thanks again for your help.

If the object implements IDisposable, you should write the code to
handle disposing. Period. To do otherwise makes one of two statements:

� I will never ever change this code or the exact platform
implementation on which it runs. It's frozen in time.

� I don't care about resource management.

Hopefully the second is not a true statement, and it's very rare that
the first is ever true.

Dealing with IDisposable objects is definitely more difficult than
managed objects. It's an unfortunate consequence of having .NET
leverage the existing OS implementation, rather than being a completely
new platform unto itself. But, that doesn't make this "trip wire"
(which implies some kind of booby trap).

It's just something you need to understand. It's no worse than needing
to understand: performance characteristics of a garbage-collected
system; the need to unsubscribe your methods from event handlers to
ensure your class will be collected; etc.

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
}
}

If the lifetime extends beyond the lifetime of any given method, then
you'll need some other mechanism. This is often as simple as the
disposable object being contained within some other disposable object,
the lifetime of which is already being managed correctly, and disposing
the first object when the containing object is disposed.

What you can't do and expect to have a useful, maintainable, correct
program is to just ignore the situation.

Pete
 |  Next  |  Last
Pages: 1 2 3 4 5 6 7
Prev: TDD - Supplying a set of rows to a single Test multiple times
Next: WIN32 DLL with C#