[Firebird-net-provider] connection pooling in a multithreading environment

[Bruce T. <BTi...@pb...>]

We encountered an error in FB 2.1.0 and 2.5.1 in some web applications:
================================================
Message: Object reference not set to an instance of an object. 
Source: FirebirdSql.Data.FirebirdClient 
Stack trace: at
FirebirdSql.Data.FirebirdClient.FbCommand.RollbackImplicitTransaction()
at FirebirdSql.Data.FirebirdClient.FbCommand.CommitImplicitTransaction()
at
FirebirdSql.Data.FirebirdClient.FbCommand.DiscardImplicitTransaction()
at
FirebirdSql.Data.FirebirdClient.FbCommand.ExecuteReader(CommandBehavior
behavior) at
FirebirdSql.Data.FirebirdClient.FbCommand.ExecuteDbDataReader(CommandBeh
avior behavior) at
System.Data.Common.DbCommand.System.Data.IDbCommand.ExecuteReader(Comman
dBehavior behavior) at
System.Data.Common.DbDataAdapter.FillInternal(DataSet dataset,
DataTable[] datatables, Int32 startRecord, Int32 maxRecords, String
srcTable, IDbCommand command, CommandBehavior behavior) at
System.Data.Common.DbDataAdapter.Fill(DataTable[] dataTables, Int32
startRecord, Int32 maxRecords, IDbCommand command, CommandBehavior
behavior) at System.Data.Common.DbDataAdapter.Fill(DataTable dataTable)
at <et-cetera>
==================================================
Similar errors were occurring too frequently from various Firebird
queries (all SELECT; no updates, deletions or insertions), some very
simple, some very complex.  We tried adding explicit transactions and
committing in lieu of implicit transaction control, to no avail.

We determined that the exception originated in the following Firebird
code:

public FbDataReader ExecuteReader(CommandBehavior behavior) 
{ 
    lock (this) 
    { 
        this.CheckCommand(); 
        try 
        { 
            this.ExecuteCommand(behavior, true); 
        } 
        catch (IscException exception) 
        { 
            this.DiscardImplicitTransaction(); 
            throw new FbException(exception.Message, exception); 
        } 
        catch 
        { 
            this.DiscardImplicitTransaction(); 
            throw; 
        } 
    } 
    this.activeReader = new FbDataReader(this, this.connection,
behavior); 
    return this.activeReader; 
} 

Specifically, the line "this.ExecuteCommand(behavior,true);".  Because
this line is enclosed within a try/catch block, the exception is
handled.  However, this alters program flow to the catch block.  Inside
the catch block, the DiscardImplicitTransaction() method is called
before the Exception is thrown.  Unfortunately, that method calls
CommitImplicitTransaction: 

internal void CommitImplicitTransaction() 
{ 
    if ((this.HasImplicitTransaction && (this.transaction != null)) &&
(this.transaction.Transaction != null)) 
    { 
        try 
        { 
            this.transaction.Commit(); 
        } 
        catch (Exception) 
        { 
            this.RollbackImplicitTransaction(); 
            throw; 
        } 
        finally 
        { 
            if (this.transaction != null) 
            { 
                this.transaction.Dispose(); 
                this.transaction = null; 
                this.implicitTransaction = false; 
            } 
            if (this.statement != null) 
            { 
                this.statement.Transaction = null; 
            } 
        } 
    } 
} 


>From the stack trace we know the line "this.transaction.Commit()" throws
a new Exception.  Because the line is encased in a try/catch block, this
exception is caught and program flow is redirected to the catch block.
The catch block calls "this.RollbackImplicitTransaction()" before
throwing the exception up the call stack.  However, before the exception
can be rethrown, this.RollbackImplicitTransation() throws the
NullReferenceException.  Here is the RollbackImplicitTransaction method:


internal void RollbackImplicitTransaction() 
{ 
    if ((this.HasImplicitTransaction && (this.transaction != null)) &&
(this.transaction.Transaction != null)) 
    { 
        int transactionCount =
this.Connection.InnerConnection.Database.TransactionCount; 
        try 
        { 
            this.transaction.Rollback(); 
        } 
        catch 
        { 
            if
(this.Connection.InnerConnection.Database.TransactionCount ==
transactionCount) 
            { 
                IDatabase database =
this.Connection.InnerConnection.Database; 
                database.TransactionCount--; 
            } 
        } 
        finally 
        { 
            this.transaction.Dispose(); 
            this.transaction = null; 
            this.implicitTransaction = false; 
            if (this.statement != null) 
            { 
                this.statement.Transaction = null; 
            } 
        } 
    } 
} 

The NullReferenceException is not handled by the Firebird DLL and is
passed up the call stack to our DLL.  Consequently, we don't see the two
original exceptions, making it next to impossible to infer what caused
them.

But I was able to finally figure it out.  I was able to reproduce the
error in a multithreaded environment.  When I locked the code which
called the DLL which uses Firebird, the error went away.  Without the
lock, the error also did not occur when our SQL DLL (issuing Firebird
queries) was called serially (sequential calls rather than parallel).
But when a bunch of threads were fired up all at once, dann war die
Hoelle los.  That is to say, one thread would bomb and take the rest
with it.  Ugly.

This gave me a hunch as to what the problem was.  I tried commenting out
the "FbConnection.ClearAllPools()" statement, and voila!  End of error.

We had started clearing all pools a few years ago because we were seeing
persistent connections on our servers, because the .NET Data Provider
pools connections even after they are closed, so that they can be reused
on the next call to the database.  The pooled connection stays "alive"
on the server until the application is closed; unfortunately, since
these were web applications, they were never truly closed.  So, when any
thread issues a "clear all pools" command, it knocks the slats right out
from under all the other threads sharing the connection pool.

The solution was simply to change our connection string from
"Pooling=True" to "Pooling=False".  That fixed it.  It rendered the
"clear all pools" statement moot, although I commented it out for good
measure, even though testing showed that it didn't hurt anything to
leave it active.  The small amount of overhead incurred by not pooling
is much preferable to bombing.

Thus, it appears that connection pooling in Firebird is mighty dangerous
in a multithreading environment.  You can, to be sure, use a lock block
around all code which issues SQL commands to Firebird, and this will
also prevent the error; but it will also create a bottleneck.  Better
just to turn connection pooling off.

Comments and additional insight would be welcome and appreciated.