Do I have any SQL leaks?

This is a Windows service that will run on about 600 machines. It is used to track the job server that a user is connected to (pushed through some kind of load balancer, not my area). I store this information in a SQL table and want to make sure that I don't have any SQL leaks.

this grabs information from the users computer (username/network name/AD Name) and requests a simple XML from the Load Balancer (which points to one of the job servers, all of which have a different value in the XML node retrieved) and then sends that information to a stored procedure which updates or inserts the information into the table, none of this happens if the application is not open.

```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Xml;
using System.Xml.XPath;
using System.Xml.Serialization;
using System.Data.Sql;
using System.Xml.Linq;
using System.Diagnostics;
using System.Data.SqlClient;
using System.Data.ProviderBase;
using System.Data.Common;
using System.Data.SqlTypes;
using System.Management;
using System.Net;
using System.Threading;

namespace AppServerTracker
{
class Tracker
{
int intConnCount;
string strServer;
string strUserName;
Process[] process = Process.GetProcessesByName("MShell");
public static string strInput = "http://Website.for.load.Balancer.com";

string strErrorMSG = "No Error"; //will show that there has been no error in the table.

private static string SQLConnectionString = Properties.Resources.ConnStage;
public SqlConnection SQLConnection = new SqlConnection(SQLConnectionString);

public Tracker()
{

}

public void Main()
{
if (process.Length > 0)
{
intConnCount = process.Length;

ManagementScope ms = new ManagementScope("\\\\.\\root\\cimv2");
ObjectQuery query = new ObjectQuery("SELECT * FROM Win32_ComputerSystem");

Your code is asymmetrical....

You open your SQLConnection inside the using block, but you close it outside in the final block.... anyway, the Close is completely redundant:


If the SqlConnection goes out of scope, it won't be closed. Therefore, you must explicitly close the connection by calling Close or Dispose. Close and Dispose are functionally equivalent.

You are already disposing the connection inside the try block (as part of the using) so it is all moot.

The SQLConnection .... how is it created??? You have it magically appearing in the using preamble.... need ... more ... information.

Why do you have commented-out old code inside the block. If you are not using the sqlCommand variable any more (since it moved to the outer using block), then kill it, don't comment out the line.

Without knowing more about what the stored procedure does, it is hard to determine whether there are any other problems, but, your code could simply look like (although as the program exits the using block the SQLConnection will be disposed. This is an asymmetrical situation... the connection was open when the block started, and we are not returning it open):

try
{
    using (SQLConnection)
    {
        SQLConnection.Open();
        using (SqlCommand TrackSproc = new SqlCommand("spServerTracking", SQLConnection))
        {
            TrackSproc.CommandType = System.Data.CommandType.StoredProcedure;
            TrackSproc.Parameters.AddWithValue("@UserName", strUserName);
            TrackSproc.Parameters.AddWithValue("@Server", strServer);
            TrackSproc.Parameters.AddWithValue("@ConnCount", intConnCount);
            TrackSproc.Parameters.AddWithValue("@MachineName", strComputerName);
            TrackSproc.Parameters.AddWithValue("@Error", strErrorMSG);
            TrackSproc.ExecuteNonQuery();
        }
    }
}
catch (Exception e)
{
    // E-mail Exception 
}

StackExchange Code Review Q#43439, answer score: 6