DataAction / AdoNetCore.AseClient

AdoNetCore.AseClient - a .NET Core DB Provider for SAP ASE
Apache License 2.0
108 stars 45 forks source link

Pooled connection leak when commands after connection open fail #148

Closed driseley closed 5 years ago

driseley commented 5 years ago

Describe the bug If a sybase database is in a state where the connection open and initial login works but subsequent commands fail, such as being offline, in LOAD DATABASE mode, or using an incorrect database name in the connection string, the internal connection pool leaks connections when pooling is enabled

To Reproduce The program below configures a connection to a non-existent database (guest1) with a connection pool of size 5. It then tries to connect and execute a query seven times.

        static void Main(string[] args)
        {
            var connectionString = $@"Data Source=localhost;Port=5000;Database=guest1;Uid=guest;Pwd=guest1234;
Pooling=true;Min Pool Size=1;Max Pool Size=5;Connection Timeout=5;ConnectionIdleTimeout=60;";

            for (int i=0; i<7; i++)
            {
                Console.WriteLine($"ATTEMPT {i}");
                try
                {
                    using (var connection = new AseConnection(connectionString))
                    {
                        connection.Open();
                        using (AseCommand command = connection.CreateCommand())
                        {
                            command.CommandText = "select 1";
                            command.ExecuteScalar();
                        }
                    }
                }
                catch (Exception e)
                {
                    Console.WriteLine(e.Message);
                }
            }
        }

Actual behaviour The connection fails 5 times with the expected error of:

Attempt to locate entry in sysdatabases for database 'guest1' by name failed - no entry found under that name. Make sure that name is entered properly.

The 6th and 7th attempts fail with:

Pool timed out trying to reserve a connection

indicating that the pool has leaked all it's connections.

ATTEMPT 0
Attempt to locate entry in sysdatabases for database 'guest1' by name failed - no entry found under that name. Make sure that name is entered properly.

ATTEMPT 1
Attempt to locate entry in sysdatabases for database 'guest1' by name failed - no entry found under that name. Make sure that name is entered properly.

ATTEMPT 2
Attempt to locate entry in sysdatabases for database 'guest1' by name failed - no entry found under that name. Make sure that name is entered properly.

ATTEMPT 3
Attempt to locate entry in sysdatabases for database 'guest1' by name failed - no entry found under that name. Make sure that name is entered properly.

ATTEMPT 4
Attempt to locate entry in sysdatabases for database 'guest1' by name failed - no entry found under that name. Make sure that name is entered properly.

ATTEMPT 5
Pool timed out trying to reserve a connection
ATTEMPT 6
Pool timed out trying to reserve a connection

Expected behavior

All 7 attempts should fail with the error:

Attempt to locate entry in sysdatabases for database 'guest1' by name failed - no entry found under that name. Make sure that name is entered properly.

Environment

Additional context

The following lines in ConnectionPool.cs:

https://github.com/DataAction/AdoNetCore.AseClient/blob/bae4753c43e2b4f4d7af2f4cc80538ab732954a7/src/AdoNetCore.AseClient/Internal/ConnectionPool.cs#L67-L74

should be wrapped in a try/catch block, releasing the connection if anything fails as the connection has been allocated from the pool but is not returned as an exception is thrown , so is not disposed of correctly

I will submit a PR with a unit test for this issue and a proposed fix

senseibaka commented 5 years ago

Good catch!

driseley commented 5 years ago

@senseibaka - Thanks for merging this. Apologies for asking again - but any chance of a swift release of this fix, we are having to do production restarts to work around this bug ( as our dbs are regularly reloaded from backups )?

senseibaka commented 5 years ago

@driseley Yeah, I'll see about finishing up #150 and try to get out a 0.16.0 soon after

senseibaka commented 5 years ago

@driseley it's out now ;)

driseley commented 5 years ago

@senseibaka and @c-j-hughes - thanks for the quick turnaround, that's awesome. Do you have any mechanism for the donation of a beer or two!?