Closing a null sql connection

Question

I have a question about closing an opened connection to a database in C#. Let's say we abandon the "using" method and use a try/catch/finally block to open and close the connection.

try 
{
    connection = new SqlConnection();
    connection.Open();
}
catch (Exception ex)
{
    // Do whatever you need with exception
}
finally
{
  1.connection.Dispose();
  2.if (connection != null)
    {
        connection.Dispose();
    }
}

My question here is what exactly happens if only 1) in this code segment occurs. The connection should always be checked for null before disposed(closed), but what if it isn't, and we try to close a connection object that's null? (I'm interested in what happens to the opened connection, can this lead to a leak)

If `connection` was null at 1 - impossible with your exact example as it is - then you'll get a `NullReferenceException` in the finally block on the `Dispose` which will propagate up. — steve16351, Sep 23 '19 at 20:00
The same thing that happens when you try to call any method on null ^^ — Devon Bessemer, Sep 23 '19 at 20:02
But what happens to the connection previously opened? Is there a possibility it stays open and leads to a connection leak? — Filip5991, Sep 23 '19 at 20:06
Unless you manually assign it to null you won't have a connection opened. If you don't close your opened connections you might run out of application pool. — André Sanson, Sep 23 '19 at 20:10
What opened connection? Are you asking what happens if you open `connection` then set `connection = null` then try to do `connection.Close()`? You'll get a `NullReferenceException` and I reckon the `SqlConnection` object that `connection` used to reference before being set to `null` will sit in memory, opened, until the garbage collector picks it up. — Joshua Robinson, Sep 23 '19 at 20:10
@JoshuaRobinson Yes, let's say i open a ```connection```, but DO NOT set ```connection=null``` and then try to do ```connection.close()```(without checking it for null). Why would i check the connection for null if i don't manually assign it to null is what i'm trying to find out — Filip5991, Sep 23 '19 at 20:17
@Filip5991 You need to check it for `null` because you're in a `finally` block and you don't know how you got there. If you try to call `Dispose` (or any method) on a `null` object, you'll get a `NullReferenceException`. Surely it would take less time to try this out than to ask the question...unless I'm missing the question here. — Rufus L, Sep 23 '19 at 20:19
Or in other words an application i'm using has a problem with the app pool as there are obviously leaks. So i was wondering if opening a ```connection``` and then closing it in the finally block without previously checking it for ```null``` could lead to those leaks — Filip5991, Sep 23 '19 at 20:20
No. It would lead to exceptions. But really, *that* should be your question, then. The question above is a little confusing, and leading to lots of "incorrect" answers because it's not clear what you're asking. — Rufus L, Sep 23 '19 at 20:22
Connection objects should be created, used, closed, and disposed as soon as possible (in other words, they should always be created in the context of a `using` block). — Rufus L, Sep 23 '19 at 20:24
@RufusL If you already need `try`/`catch` blocks (which is debatable... ime, it's usually better to let those exceptions bubble up a level), `finally` offers similar safety and conciseness. I prefer `using`, but it's too strong to say the `finally` is _wrong_. — Joel Coehoorn, Sep 23 '19 at 20:36
@JoelCoehoorn True. I didn't mean to imply that finally is wrong (after all that's what a using block generates behind the scenes anyway). What I meant to say (by implication, I guess), is that the problem is most likely that connections are probably not being disposed, which happens sometimes when they are shared across a class, but not something that would happen in the code presented in the question. Something for OP to check out: https://softwareengineering.stackexchange.com/questions/142065/creating-database-connections-do-it-once-or-for-each-query — Rufus L, Sep 23 '19 at 20:56

Joel Coehoorn · Answer 1 · 2019-09-23T20:38:35.033

In the question, if connection.Dispose() is called on a null connection without the check, you cause a new exception that has not been handled, and everything that goes with that. Additionally, if this was done because the connection variable was set to null before the finally block without ensuring the connection was closed, it is possible to leak that open connection ... though as a practical matter it's rare to see code in the wild that knows to use a finally but also wants to set the connection to null.

Granting the lack of using (which is suspect, but whatever), I prefer this pattern:

finally
{
    if (connection is IDisposable) connection.Dispose();
}

This still protects against null values* in the connection object, and is a closer mimic for what the using pattern was already doing behind the scenes.

Note, however, you're still missing one important feature of a using block, which is protection against something assigning null to your variable before the block closes. using blocks also protect your connection in a separate (hidden) variable, so you're sure you still have a valid reference at the end of the block (this is new in C# 8).

*(For completeness, the author of that linked tweet is the lead on the C# compiler team, so he should know, and he's not being ironic).

I'm using ```using```, i'm just hypothetically interested in what's happening if a null connection is closed — Filip5991, Sep 23 '19 at 20:01
*"what's happening if a null connection is closed"* is impossible. A null connection cannot be closed. — Rufus L, Sep 23 '19 at 20:17
`Dispose()` should never be called directly but by letting the compiler doing that using the `using` keyword. — , Sep 23 '19 at 20:18
@OlivierRogier I mostly agree (hence the "which is suspect" part of the answer), but **never** is a strong word. — Joel Coehoorn, Sep 23 '19 at 20:23

Closing a null sql connection

1 Answers1