Do I already close all query and connections properly?

Question

Hi All,

I'm struggling with this exception "System.InvalidOperationException: Can't close, connection is in state Connecting at Npgsql.NpgsqlConnection.Close(Boolean async)"

I suspect there are memory leak of my database query or connections, I curious, as I'm still in learning process, so I'm not sure about my query and database connection below, is does it closed properly?

Do I already close all query and connections properly?

When querying

-------------

        try  
        {  
            using (NpgsqlConnection conn = new NpgsqlConnection(Models.AppSettings.PG_SQL.Connection_String))  
            {  
                try  
                {  
                    string sql = "select * from test where pid=@Pid";  

                    using (var dr = await conn.ExecuteReaderAsync(sql, new { Pid = pid }))  
                    {  
                        if (await dr.ReadAsync())  
                        {  
                            // Do some processes here..blah blah  
                        }  
                    }  
                }  
                catch (Exception e)  
                {  
                    // Save to log  
                }  
                finally { }  
            }  
        }  
        catch (Exception e)  
        {  
            // Save to log  
        }  
        finally { }  

When adding/updating

--------------------

        try  
        {  
            using (NpgsqlConnection conn = new NpgsqlConnection(Models.AppSettings.PG_SQL.Connection_String))  
            {  
                try  
                {  
                    string sql = "Insert Into test (learn_code, learn_name) Values " +  
                        "(@Learn_Code, @Learn_Name) Returning pid";  

                    ret = conn.ExecuteScalar<long>(sql, new  
                    {  
                        Learn_Code = learnCode,  
                        Learn_Name = learnName  
                    });  

                }  
                catch (Exception e) { // Save Log }  
                finally { }  
            }  
        }  
        catch (Exception e) { // Save Log }  
        finally { }  

Answer 1

Your code is properly cleaning up the resources. Every type that implements IDisposable should be wrapped in a using statement. In your case that is the connection and data reader types.

As an aside you can get rid of the finally blocks as they serve no purpose. Additionally in your first block of code you have an inner try-catch wrapped in an outer try-catch. There is no reason to do this. Ideally the try-catch stuff should be in a higher level function even for logging. This low level DB code has no way of knowing whether the higher level code is going to handle the exception or not so logging it at this point is redundant. But nevertheless here's a simplified version of your otherwise fine code.

   try  
   {  
       using (NpgsqlConnection conn = new NpgsqlConnection(Models.AppSettings.PG_SQL.Connection_String))  
       {  
           string sql = "select * from test where pid=@Pid";  
           using (var dr = await conn.ExecuteReaderAsync(sql, new { Pid = pid }))  
           {  
               if (await dr.ReadAsync())  
               {  
                   // Do some processes here..blah blah  
               }  
           }  
       }  
   }  
   catch (Exception e)  
   {  
       // Save to log  
   }  

And if you're using .NET 5+ then the code can be simplified even further.

   try  
   {  
       using var conn = new NpgsqlConnection(Models.AppSettings.PG_SQL.Connection_String);  
     
       string sql = "select * from test where pid=@Pid";  
         
       using var dr = await conn.ExecuteReaderAsync(sql, new { Pid = pid });  
       if (await dr.ReadAsync())  
       {  
           // Do some processes here..blah blah              
       }  
   }  
   catch (Exception e)  
   {  
       // Save to log  
   }  

The exception you're getting seems to indicate that you're attempting to close/dispose of the connection while it is in the middle of trying to connect. Looking at the reported issues over time on this type it seems like they have issues with state management in general. My gut instinct is that the connection is failing during your ExecuteReaderAsync call and when the connection is disposed it fails because the connection state is bad. This is outside your control. You could try opening the connection in advance and see if you get past the exception.

   try  
   {  
       using var conn = new NpgsqlConnection(Models.AppSettings.PG_SQL.Connection_String);  
       await conn.OpenAsync();  
     
       string sql = "select * from test where pid=@Pid";  
         
       using var dr = await conn.ExecuteReaderAsync(sql, new { Pid = pid });  
       if (await dr.ReadAsync())  
       {  
           // Do some processes here..blah blah              
       }  
   }  
   catch (Exception e)  
   {  
       // Save to log  
   }  

I should also point out that once the reader is created (inside ExecuteReaderAsync) then it owns the connection and will clean it up. But it should be harmless to clean up a connection multiple times. If it isn't then the code is badly written and should be reported to the developer.

If explicitly opening the connection doesn't resolve the issue then create a simple wrapper type around the connection that closes the connection and silently eats any exceptions it throws. This will resolve your issue until the developer fixes their code.