0

I have a class in C# called SQLServerConnection that holds a SqlConnection object. The class connects to a SQLServer with the SqlConnection instance. The class has a destructor as follows:

~SQLServerConnection()
{
    dbConnection.Close();
}

Upon program completion, an error is thrown at dbConnection.Close() with the following error:

System.InvalidOperationException: 'Internal .Net Framework Data Provider error 1.'

Any idea why this is happening? If I call Close() normally in the main class this works fine but only throws an error if done in the destructor.

Also, is it bad practice to close connections in destructors like this?

Richardissimo
  • 5,596
  • 2
  • 18
  • 36
redTurtle
  • 5
  • 2
  • What language? I'm assuming C#, since we're talking .NET, but tag appropriately -- it might be C++/CLI for all we know. Be aware that C# does not *have* destructors; if this is C# then what you've got there is a finalizer, which unfortunately shares syntax, but is quite different from a C++ destructor. Read up on when you use finalizers -- hint: very rarely, and certainly not in this case. – Jeroen Mostert Jun 29 '18 at 15:52
  • 1
    You should close and dispose your connection immediately after you run a query every time. If you try to keep a connection during the entire time a program is running you are on a path of destruction. Your connection pool will get consumed and then nobody can get a connection and your application will be rendered useless. You should read up on how to properly handle data connections. – Sean Lange Jun 29 '18 at 15:54
  • Sorry for the confusion, yes this is C# – redTurtle Jun 29 '18 at 16:00
  • Please see https://stackoverflow.com/a/61131/4180382 – Ole EH Dufour Jun 29 '18 at 16:10
  • @redTurtle The request from Jeroen was to "tag appropriately", i.e. [edit] your question to add the C# tag. Adding to the title is a good idea; but the tags are what will get your question seen by the right people. I've fixed it for you. – Richardissimo Jun 29 '18 at 17:35
  • do you have inner exception? – Viacheslav Yankov Jun 29 '18 at 17:38
  • You don't need to use finalizer(destructor) in this case, just use Dispose pattern. – Viacheslav Yankov Jun 29 '18 at 17:43
  • Finalizers should only free unmanaged resources. You're not supposed to access a managed object from a finalizer, which is one of the reasons writing a *proper* finalizer is so difficult. Ditch the finalizer and switch to the dispose pattern. – pinkfloydx33 Jun 30 '18 at 09:07

1 Answers1

2

It looks like you want your class to encapsulate the out-of-the-box SqlConnection class, since it holds an instance of SqlConnection in dbConnection which I assume is a field on the class instance.

The thing is that SqlConnection is IDisposable. So if your class wants to hold an instance of that in a field, that means your class should also be IDisposable. From Microsoft's CodeAnalysis rules (literally, it is rule 1001), see Types that own disposable fields should be disposable.

You might also like to note the line on that page saying:

If the class does not directly own any unmanaged resources, it should not implement a finalizer.

Richardissimo
  • 5,596
  • 2
  • 18
  • 36