0

I have code like this:

~MyClass() {
    try {
        if (Database.Exists(_connectionString))
        {
            Database.Delete(_connectionString);
        }
    } catch { }
}

Database is a static class of Entity Framework, whereas _connectionString is a private readonly string set by the ctor. The idea is that if someone forgot to Dispose the class, we still clean state (in my case, this is part of an integration test where the test runner doesn't call Dispose if there's an unhandled exception in the test, so it's not something I can fix on my side)

However, Finalizers are generally not supposed to call class members because they might be disposed already, so if I end up in a scenario where the _connectionString is already collected, I might have a problem.

Is there a way to do this safely (e.g,, using some sort of GC.KeepAlive construct?)

Michael Stum
  • 177,530
  • 117
  • 400
  • 535
  • 2
    That try catch also smells in a finalizer... :( – Simon Whitehead Jun 27 '13 at 07:47
  • 1
    @Justin: He already does implement `IDisposable`: *"The idea is that if someone forgot to Dispose the class, we still clean state"* – Daniel Hilgarth Jun 27 '13 at 08:01
  • @Simon The Try..Catch is to make sure the Finalizer doesn't throw an Exception, since that would resurrect the object. – Michael Stum Jun 27 '13 at 08:02
  • 1
    @Justin Thanks - Lasse V. Karlsen's answer is perfect. Voted to close. – Michael Stum Jun 27 '13 at 08:05
  • 1
    @MichaelStum Actually I think that an exception in a finalizer will immediately terminate the entire application without running any further finalizers! (So you really don't want to let exceptions escape from finalizers) – Matthew Watson Jun 27 '13 at 08:06
  • 1
    Here's a tip though. I would probably not implement heavy lifting in a finalizer. If there is any chance at all that the finalizer code will hang, wait, crash, whatnot, you risk the finalizer thread going stale/down, and then you *really* have problems. Instead if this is a pattern you need to solve, I would "queue up" the database for deletion and handle the actual deletion somewhere else. Though, the best way would of course be to make sure you dispose of the object everywhere. – Lasse V. Karlsen Jun 27 '13 at 08:17
  • 1
    Exceptions in finalizers will not (at least not in older .NET versions, unsure about 4/4.5) terminate the application, but they will in many cases terminate the finalizer thread, which is worse, the *freachable* list now grows unbounded, resulting in memory problems down the line. – Lasse V. Karlsen Jun 27 '13 at 08:18
  • @DanielHilgarth Fair enough, but [finalizers aren't free](http://blogs.msdn.com/b/cbrumme/archive/2004/02/20/77460.aspx) - they do come with a performance penalty. Finalizers are designed for cleaning up **critical** unmanaged resources (like handles, database connections, or other things that would cause serious damage if left open) rather than as the go-to place for tidying up. I don't know what `Database.Delete` is doing but I'd recommend implementing `IDisposable` *first*, and then refactoring so that it is also called from the finalizer if its deemed to be absolutely necessary. – Justin Jun 27 '13 at 08:56
  • 1
    My above comment about finalizers and the finalization thread is not correct, but I am unsure what is correct as well. Testing in LINQPad did not crash the finalizer thread, but in a console program terminated the program, so I'm not sure about this. I'll leave the comment but please disregard most of it. The conclusion still stands, **don't crash in the finalizer**. – Lasse V. Karlsen Jun 27 '13 at 08:59

1 Answers1

0

Edit

As pointed out, below was not a direct answer to the question so....

Yes it is safe to access the member variables (in this particular example - assuming the connection string is a string) and in addition I recommend you check out the IDisposable pattern.

Original

You should implement the IDisposable pattern:

http://msdn.microsoft.com/en-us/library/system.idisposable.aspx

This will ensure that the Dispose method is always called and you can safely access the member variables.

gareththegeek
  • 2,362
  • 22
  • 34
  • 1
    -1: Implementing `IDisposable` *does not* guarantee that the `Dispose` method is always called. In fact, the client of the class has to call that method explicitly. – Daniel Hilgarth Jun 27 '13 at 08:00
  • Dispose isn't guaranteed to be called. If callers don't wrap this in a using statement, Dispose will never be called. The Garbage Collector doesn't call Dispose (see http://stackoverflow.com/questions/1691846/does-garbage-collector-call-dispose ). Only the Finalizer will be called by the Garbage Collector (unless there is a catastrophic failure of the app of course). The point of my finalizer is to ensure cleanup because I know I have a caller I don't control that doesn't call Dispose. – Michael Stum Jun 27 '13 at 08:01
  • When using the IDisposable pattern, the Dispose method is always called, the finalizer calls the dispose method... – gareththegeek Jun 27 '13 at 08:03
  • 1
    @gareththegeek no, it explicitly ***does not*** do that, unless *your code* calls it; it is the `using` pattern, or explicit calls to `Dispose()`, that call `Dispose()` - *nothing more* – Marc Gravell Jun 27 '13 at 08:04
  • 3
    @gareththegeek That begs question though. Instead of asking "Can I access a string in my finalizer" the question is now "Can I access a string in a method called by my finalizer" :) Disposable has absolutely nothing to do with this. – Michael Stum Jun 27 '13 at 08:04
  • 1
    Also see the key point in the question: "The idea is that if someone forgot to Dispose the class, " - the OP is *already* implementing `IDisposable` – Marc Gravell Jun 27 '13 at 08:05
  • @Marc Gravell - hence why I recommended using the IDisposable PATTERN i.e. calling Dispose from the finalizer and GC.SupressFinalizer from the public Dispose method. – gareththegeek Jun 27 '13 at 08:06
  • @gareththegeek: That wouldn't help. If implementing it according to that pattern, the private `Dispose` method only disposes of *unmanaged* resources. It doesn't dispose managed ones, because of the exact context of this question: What is allowed to be called from a finalizer and what not? – Daniel Hilgarth Jun 27 '13 at 08:10
  • Your edit just makes your answer worse: *"Yes it is safe to access the member variables"*. That's just not true. It totally depends on the actual member. – Daniel Hilgarth Jun 27 '13 at 08:11
  • 1
    To be pedantic, it is safe to access the member field, for instance, to do a null-comparison, ie. this is entirely OK regardless of the object type: `if (memberField == null) { ... }`. Is it safe to access the object it refers to, ie. dereference the member field and access members on the object? Depends on the object. If it has been finalized, probably not, but it depends really. A string field on that object still lives and is safe to access. Something that accesses something that the finalizer cleaned up might throw an exception or worse though. *It depends!* – Lasse V. Karlsen Jun 27 '13 at 08:23