[Mono-dev] Suggested fix for dropped SQL connection

[Neale Ferguson]

While executing SQL Transactions it's possible that the connection may be
lost and a read or write operation will fail. For example, at the moment
Tds.ExecuteQuery will catch the I/O exception (resulting from
TdsComm.SendPhysicalPacket getting an error doing a write) and set connected
= false. The application program will detect this error and attempt to close
down which involves the various Dispose methods being invoked and doing
their thing. Currently, the SqlTransaction Dispose method will invoke
Rollback(). Now, when a transaction hasn't been completed/committed but the
connection is still up, this is a valid thing to do. However, if the
connection has been lost then Rollback() will in turn lead to a
Tds.ExecuteQuery which involves sending a packet to the server side. With a
dead connection this will fail and we'll get a heap of other exceptions.

As Tds.ExecuteQuery is detecting the failed connection and setting connected
to false, I propose the following:

--- a/mcs/class/System.Data/System.Data.SqlClient/SqlTransaction.cs
+++ b/mcs/class/System.Data/System.Data.SqlClient/SqlTransaction.cs
@@ -158,8 +158,9 @@ namespace System.Data.SqlClient
                        if (!isOpen)
                                throw ExceptionHelper.TransactionNotUsable
(GetType ());
 
-                       connection.Tds.Execute (String.Format ("IF
@@TRANCOUNT > 0 ROLLBACK TRANSACTION {0}",
-                  
transactionName));
+                       if (connection.Tds.IsConnected)
+                               connection.Tds.Execute (String.Format ("IF
@@TRANCOUNT > 0 ROLLBACK TRANSACTION {0}",
+                  
transactionName));
                        isOpen = false;
                        connection.Transaction = null;
                        connection = null;
------------------------------------------------------------

A bit more background...

Here's where Tds.ExecuteQuery is noticing the failed connection:

                protected void ExecuteQuery (string sql, int timeout, bool
wantResults)
                {
                        InitExec ();

                        Comm.StartPacket (TdsPacketType.Query);
                        Comm.Append (sql);
                        try {
                                Comm.SendPacket ();
                                CheckForData (timeout);
                                if (!wantResults)
                                        SkipToEnd ();
                        } catch (IOException ex) {
                                connected = false;
                                throw new TdsInternalException ("Server
closed the connection.", ex);
                        }
                }


The fix has been tested with the application that was previously failing
with:

Drms.Server.Oas.Persistence.Tests.DAOTests.TestLargeByteArrayFieldPersis
tence
System.ObjectDisposedException: The object was used after being
disposed.
Source: System
Target Site: Void CheckDisposed()
Stack Trace: at System.Net.Sockets.NetworkStream.CheckDisposed ()
[0x0001c] in
/home/monosrc/mono/mcs/class/System/System.Net.Sockets/NetworkStream.cs:
451
at System.Net.Sockets.NetworkStream.Write (System.Byte[] buffer, Int32
offset, Int32 size) [0x00000] in
/home/monosrc/mono/mcs/class/System/System.Net.Sockets/NetworkStream.cs:
421
at Mono.Data.Tds.Protocol.TdsComm.SendPhysicalPacket (Boolean
isLastSegment) [0x000a2] in
/home/monosrc/mono/mcs/class/Mono.Data.Tds/Mono.Data.Tds.Protocol/TdsCom
m.cs:761
at Mono.Data.Tds.Protocol.TdsComm.SendPacket () [0x0001f] in
/home/monosrc/mono/mcs/class/Mono.Data.Tds/Mono.Data.Tds.Protocol/TdsCom
m.cs:736
at Mono.Data.Tds.Protocol.Tds.ExecuteQuery (System.String sql, Int32
timeout, Boolean wantResults) [0x0001e] in
/home/monosrc/mono/mcs/class/Mono.Data.Tds/Mono.Data.Tds.Protocol/Tds.cs
:564

Neale