Deterministic finalization -- how will clients of my stuff cope?

[Ted Miller]

Hi folks.

So I'm in the middle of porting a large (previously COM-based) imaging
library to .Net. The clients of this library are VB programmers within the
company I work for. Their code will be getting ported to .Net at some point
in the near future too.

In the .Net world, many of my objects are forced to implement the
IDisposable+Finalizer pattern because they hold onto unmanaged resources or
represent resources where deterministic release is an absolute requirement.
For example, image data lives in memory blocks allocated with VirtualAlloc.
Image files are read and written using Stream-derived classes.

I'm just kinda pondering how the need for IDisposable is going to impact the
other guys in the company. This basically adds a programming requirement
that did not exist before in the environment that these guys are used to
(VB6+COM). For example if the guy building VB-based image objects on top of
my classes forgets to call Dispose on my image objects at the right times,
several to several dozen MB of memory allocated with VirtualAlloc will
remain committed, and, since that memory is unmanaged, this will not help
trigger appropriately timed GCs. Yes, that memory will get cleaned up when
my object is finalized, but for interactive operations where many images
come and go in a short period of time, this is a killer.

What I am thinking is that it is going to be a continual battle to get these
guys to into the habit of calling Dispose and hopefully of using try/finally
(whatever the equivalent is in VB). Also, they will need to implement
IDisposable on any of their own classes that hold refs to my objects.

I think this is a disaster waiting to happen. VB guys are in general just
not used to this sort of programming model (please no flames, when I say "VB
programmers" I mean "the VB programmers at my company" -- who mostly have no
clue about system-level programming issues). They will resist dealing with
IDisposable and the right things just won't happen when their code is first
ported. "IDisposable-correctness" is a bit like "const-correctness" in
C++ -- since it tends to permeate and percolate upwards through the design,
it is a real pain to retrofit.

I am wondering if there is anything I can do to help with this problem. I
mean, it seems to me that the compiler and the CLR know when a reference is
being "released" -- why isn't there a mechanism whereby an object can
receive notifications that references have been added or removed to itself?
If I had that, I could consider implementing some kind of auto-Dispose. Sort
of like an "operator Referenced" and "operator Released" -- maybe this is
the same thing as trying to overload operator= (which can't be done in C#)?

class MyClass {

private int RefCount;

void op_Referenced()
{
RefCount++;
}

void op_Released()
{
if(--RefCount == 0) Dispose();
}
}

void somecode()
{
MyClass p = new MyClass(); // call p.op_Referenced
p = null; // call p.op_Released
}

Am I missing anything existing in VS .Net 2003 that might help me here?
Maybe VB.Net does this already and this isn't the issue I think it is gonna
be? Am I just nuts?

 

[Jeff Louie]

Ted... Since no one has responded, here is my twisted code that will
crash the application on error! Remember, just because it can be done
does not mean it should be done!

class Debug
{
public static readonly bool isDebug= true;
}
/// <summary>
/// Summary description for Class1.
/// </summary>
///
class Class1
{
bool isDisposed= false;
~Class1()
{
if (!isDisposed)
{
if (Debug.isDebug){ throw new Exception();}
else {
System.Console.WriteLine("Log this.");
System.Console.ReadLine();
}
}
}
public bool MyDispose()
{
if (!isDisposed)
{
isDisposed= true;
return true;
}
else return false;
}
/// <summary>
/// The main entry point for the application.
/// </summary>
[STAThread]
static void Main(string[] args)
{
//
// TODO: Add code to start application here
//
Class1 c= new Class1();
c.MyDispose();
c= null;
c= new Class1();
c= null;
}
}
Regards,
Jeff

What I am thinking is that it is going to be a continual battle to get

these guys to into the habit of calling Dispose<

 

[Ted Miller]

Well, that's an interesting approach, actually -- raise an exception or
crash or whatever if an object actually gets finalized, the idea being to
get someone's attention and tell them, hey! you forgot to finalize!

Ted... Since no one has responded, here is my twisted code that will
crash the application on error! Remember, just because it can be done
does not mean it should be done!

class Debug
{
public static readonly bool isDebug= true;
}
/// <summary>
/// Summary description for Class1.
/// </summary>
///
class Class1
{
bool isDisposed= false;
~Class1()
{
if (!isDisposed)
{
if (Debug.isDebug){ throw new Exception();}
else {
System.Console.WriteLine("Log this.");
System.Console.ReadLine();
}
}
}
public bool MyDispose()
{
if (!isDisposed)
{
isDisposed= true;
return true;
}
else return false;
}
/// <summary>
/// The main entry point for the application.
/// </summary>
[STAThread]
static void Main(string[] args)
{
//
// TODO: Add code to start application here
//
Class1 c= new Class1();
c.MyDispose();
c= null;
c= new Class1();
c= null;
}
}
Regards,
Jeff

What I am thinking is that it is going to be a continual battle to get

these guys to into the habit of calling Dispose<

 

[Jeff Louie]

He He He. The devil made me write that code. I repent .

Regards,
Jeff

Well, that's an interesting approach, actually -- raise an exception or

crash or whatever if an object actually gets finalized, the idea being
to
get someone's attention and tell them, hey! you forgot to finalize!<

 

[Eric Johannsen]

I was going to suggest the same thing - but maybe not throw the exception
but just log the offending class info. After all the exception will seem to
happen at random times and may be more confusing than writing to a log file.

The way I would set it up is:

1) Tell developers to check the log file. Some will, some won't.
2) Do code reviews prior to release (we always do this for all new code).
Not just for this issue - it's generally a great idea.
3) Tell QA to check the log file and reject a build if there are any entries
indicating IDisposable was not properly used

Eric

Ted... Since no one has responded, here is my twisted code that will
crash the application on error! Remember, just because it can be done
does not mean it should be done!

class Debug
{
public static readonly bool isDebug= true;
}
/// <summary>
/// Summary description for Class1.
/// </summary>
///
class Class1
{
bool isDisposed= false;
~Class1()
{
if (!isDisposed)
{
if (Debug.isDebug){ throw new Exception();}
else {
System.Console.WriteLine("Log this.");
System.Console.ReadLine();
}
}
}
public bool MyDispose()
{
if (!isDisposed)
{
isDisposed= true;
return true;
}
else return false;
}
/// <summary>
/// The main entry point for the application.
/// </summary>
[STAThread]
static void Main(string[] args)
{
//
// TODO: Add code to start application here
//
Class1 c= new Class1();
c.MyDispose();
c= null;
c= new Class1();
c= null;
}
}
Regards,
Jeff

What I am thinking is that it is going to be a continual battle to get

these guys to into the habit of calling Dispose<

 

[Ted Miller]

Well, I was playing around with this a little, and there's a StackTrace
class that can be wrapped inside a class that gets embedded in my objects to
hamdle this, including printing out a complete stack trace of where the
object that wasn't disposed was created. This is looking like a pretty good
idea.

I was going to suggest the same thing - but maybe not throw the exception
but just log the offending class info. After all the exception will seem to
happen at random times and may be more confusing than writing to a log file.

The way I would set it up is:

1) Tell developers to check the log file. Some will, some won't.
2) Do code reviews prior to release (we always do this for all new code).
Not just for this issue - it's generally a great idea.
3) Tell QA to check the log file and reject a build if there are any entries
indicating IDisposable was not properly used

Eric

Ted... Since no one has responded, here is my twisted code that will
crash the application on error! Remember, just because it can be done
does not mean it should be done!

class Debug
{
public static readonly bool isDebug= true;
}
/// <summary>
/// Summary description for Class1.
/// </summary>
///
class Class1
{
bool isDisposed= false;
~Class1()
{
if (!isDisposed)
{
if (Debug.isDebug){ throw new Exception();}
else {
System.Console.WriteLine("Log this.");
System.Console.ReadLine();
}
}
}
public bool MyDispose()
{
if (!isDisposed)
{
isDisposed= true;
return true;
}
else return false;
}
/// <summary>
/// The main entry point for the application.
/// </summary>
[STAThread]
static void Main(string[] args)
{
//
// TODO: Add code to start application here
//
Class1 c= new Class1();
c.MyDispose();
c= null;
c= new Class1();
c= null;
}
}
Regards,
Jeff

What I am thinking is that it is going to be a continual battle to get

these guys to into the habit of calling Dispose<

 

[Jeff Louie]

FWIW, in debug mode, an unhandled exception is thrown and the
debugger takes you directly to the line of offending code. You can then
step over the exception.

Regards,
Jeff

 

[Dave]

I think there's a lot of merit to this approach. The only change I would
make would be to think about adding a line to disable the exception when the
appdomain is unloading. Unless you can know that the Dispose should always
be called prior to the appdomain's termination you may get false exceptions.

~Class1()
{
if ( !AppDomain.CurrentDomain.IsFinalizingForUnload )
{
/// stuff goes here
}
}

 

[Ted Miller]

Thanks, and thanks to everyone else who took the time to post -- I think I
have some viable options now, to *force* the VB guys into learning how to do
the right thing.

--

 

[Ted Miller]

I am putting this whole thing into practice. I created a
DisposableTrackedObject class that creates a StackTrace instance in its
constructor. My objects will embed these, i.e.,

class MyClass : IDisposable {
private DisposableTrackedClass m_Track = new DisposableTrackedClass();

Dispose() {
//...
m_Track.Dispose();
//...
}
}

The finalizer for DisposableTrackedClass uses the stack trace it gathered
during construction to print out diagnostic info, etc (I am leaning towards
exceptions and logging). There is just one small glitch: the finalizer needs
to access the StackTrace, which is a non-static member variable. This is a
big no-no since it might have been collected already. To get around this, I
use GCHandle to hold an extra reference to the StackTrace object, thus
preventing it from being collected.

class DisposableTrackedObject : IDisposable
{
private StackTrace m_StackTrace;
private GCHandle m_StackTraceRef;
private int m_Disposed;

public DisposableTrackedObject()
{
m_StackTrace = new StackTrace(0);
m_StackTraceRef = GCHandle.Alloc(m_StackTrace);
}

public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}

~DisposableTrackedObject()
{
Dispose(false);
}

private void Dispose(bool Disposing)
{
if(System.Threading.Interlocked.CompareExchange(ref
m_Disposed,1,0) == 0) {
//
// Skip a frame to ignore this class.
//
if(st.FrameCount > 1) {
//
// The next frame up is the one where this object
was
// constructed -- which is the name of the class
that was not
// disposed.
//
string TrackedObjectName =
st.GetFrame(1).GetMethod().DeclaringType.FullName;

//
// Write object name and stack trace.
//
Console.WriteLine("Object {0} not properly
disposed!",TrackedObjectName);
if(st.FrameCount > 2) {
Console.WriteLine("Trace follows --");
for(int i=2; i<st.FrameCount; i++) {
MethodBase m= st.GetFrame(i).GetMethod();

Console.WriteLine("{0}.{1}",m.DeclaringType.FullName,m.Name);
}
}
Console.Out.Flush();
}
m_StackTrace = null;
m_StackTraceHandle.Free();
}
}
}

 

[Jon Skeet [C# MVP]]

I am putting this whole thing into practice. I created a
DisposableTrackedObject class that creates a StackTrace instance in its
constructor. My objects will embed these, i.e.,

class MyClass : IDisposable {
private DisposableTrackedClass m_Track = new DisposableTrackedClass();

Dispose() {
//...
m_Track.Dispose();
//...
}
}

The finalizer for DisposableTrackedClass uses the stack trace it gathered
during construction to print out diagnostic info, etc (I am leaning towards
exceptions and logging). There is just one small glitch: the finalizer needs
to access the StackTrace, which is a non-static member variable. This is a
big no-no since it might have been collected already.

No, it won't have been collected. It may have been *finalized* already,
but it can't have actually been collected, because you still have a
reference to it. The question is whether or not its finalizer does
anything significant, if it even has one. I rather suspect it doesn't,
given that it doesn't implement IDisposable.

 

[Ted Miller]

You are right (and thanks for pointing out my terminology error). My point
was simply that holding an explicit GCHandle to the StackTrace object means
that the design adheres to the general recommendations of not messing with
managed member objects during finalization.

This little guy has already caught 3 places in my own code where I wasn't
disposing correctly -- very useful indeed.

I can already see the VB guys whining and moaning about trying to get around
the exceptions and such since they are absolutely not used to this sort of
enforced discipline. Heh, heh -- better get used to it!