2004-09-11 Francisco Figueiredo Jr. <fxjrlists@yahoo.com.br>
authorFrancisco Figueiredo Jr. <fxjr@mono-cvs.ximian.com>
Sat, 11 Sep 2004 20:17:12 +0000 (20:17 -0000)
committerFrancisco Figueiredo Jr. <fxjr@mono-cvs.ximian.com>
Sat, 11 Sep 2004 20:17:12 +0000 (20:17 -0000)
* Npgsql/NpgsqlConnection.cs:
Dispose: Improved implementation. Also added warning logging for
NpgsqlConnection leakings.
Close: Fixed code to allow calling many times even when being disposed.
* Npgsql/NpgsqlConnection.resx:
Added new warning logging message for NpgsqlConnection leakings.
* Npgsql/NpgsqlConnector.cs:
IsValid: new method to check if Connector is still valid.
* Npgsql/NpgsqlConnectorPool.cs:
GetPooledConnector: Improved to check validity of Connector.
FixPoolCountBecauseOfConnectionDisposeFalse: new method to fix the connection pool count
because NpgsqlConnection leaking. This way, uses doesn't run out of connections from pool.

svn path=/trunk/mcs/; revision=33757

mcs/class/Npgsql/ChangeLog
mcs/class/Npgsql/Npgsql/NpgsqlConnection.cs
mcs/class/Npgsql/Npgsql/NpgsqlConnection.resx
mcs/class/Npgsql/Npgsql/NpgsqlConnector.cs
mcs/class/Npgsql/Npgsql/NpgsqlConnectorPool.cs

index b59d282c650e3e83f84fadc0819090f1822ee068..fefc6c31413c9c68fb19a3b06443d3546a545090 100644 (file)
@@ -1,3 +1,21 @@
+2004-09-11  Francisco Figueiredo Jr.  <fxjrlists@yahoo.com.br>
+
+       * Npgsql/NpgsqlConnection.cs: 
+               Dispose: Improved implementation. Also added warning logging for 
+               NpgsqlConnection leakings.
+               Close: Fixed code to allow calling many times even when being disposed.
+       * Npgsql/NpgsqlConnection.resx:
+               Added new warning logging message for NpgsqlConnection leakings.
+       * Npgsql/NpgsqlConnector.cs:
+               IsValid: new method to check if Connector is still valid.
+       * Npgsql/NpgsqlConnectorPool.cs:
+               GetPooledConnector: Improved to check validity of Connector.
+               FixPoolCountBecauseOfConnectionDisposeFalse: new method to fix the connection pool count
+               because NpgsqlConnection leaking. This way, uses doesn't run out of connections from pool.
+
+
+
+
 2004-09-07  Francisco Figueiredo Jr.  <fxjrlists@yahoo.com.br>
 
        * NpgsqlException.cs:  Added support for deserialization of NpgsqlException.
index 12da230b27329ff5e69a5e1fd950cfa4737149dc..9d3dbc21723153407848711e76d42cc39cc0f379 100755 (executable)
@@ -239,9 +239,12 @@ namespace Npgsql
             {
                 CheckNotDisposed();
 
-                if (connector != null) {
+                if (connector != null)
+                {
                     return connector.State;
-                } else {
+                }
+                else
+                {
                     return ConnectionState.Closed;
                 }
             }
@@ -335,7 +338,8 @@ namespace Npgsql
 
             CheckConnectionOpen();
 
-            if (connector.Transaction != null) {
+            if (connector.Transaction != null)
+            {
                 throw new InvalidOperationException(resman.GetString("Exception_NoNestedTransactions"));
             }
 
@@ -386,7 +390,7 @@ namespace Npgsql
 
             Close();
 
-            connection_string[ConnectionStringKeys.Database] = dbName;            
+            connection_string[ConnectionStringKeys.Database] = dbName;
 
             Open();
         }
@@ -397,19 +401,20 @@ namespace Npgsql
         /// </summary>
         public void Close()
         {
-            CheckNotDisposed();
-
-            if (connector == null) {
-                return;
-            }
+            if (!disposed)
+            {
+                NpgsqlEventLog.LogMethodEnter(LogLevel.Debug, CLASSNAME, "Close");
 
-            NpgsqlEventLog.LogMethodEnter(LogLevel.Debug, CLASSNAME, "Close");
+                if (connector != null)
+                {
 
-            connector.Notification -= NotificationDelegate;
-            connector.Notice -= NoticeDelegate;
+                    connector.Notification -= NotificationDelegate;
+                    connector.Notice -= NoticeDelegate;
 
-            NpgsqlConnectorPool.ConnectorPoolMgr.ReleaseConnector(this, connector);
-            connector = null;
+                    NpgsqlConnectorPool.ConnectorPoolMgr.ReleaseConnector(this, connector);
+                    connector = null;
+                }
+            }
         }
 
         /// <summary>
@@ -444,9 +449,27 @@ namespace Npgsql
         /// <b>false</b> when being called from the finalizer.</param>
         protected override void Dispose(bool disposing)
         {
-            Close();
-            base.Dispose (disposing);
-            disposed = true;
+
+            if (!disposed)
+            {
+                if (disposing)
+                {
+                    NpgsqlEventLog.LogMethodEnter(LogLevel.Debug, CLASSNAME, "Dispose");
+                    Close();
+                }
+                else
+                {
+                    if (State != ConnectionState.Closed)
+                    {
+                        NpgsqlEventLog.LogMsg(resman, "Log_ConnectionLeaking", LogLevel.Debug);
+                        NpgsqlConnectorPool.ConnectorPoolMgr.FixPoolCountBecauseOfConnectionDisposeFalse(this);
+                    }
+                }
+
+                base.Dispose (disposing);
+                disposed = true;
+
+            }
         }
 
         /// <summary>
@@ -467,10 +490,11 @@ namespace Npgsql
             CheckNotDisposed();
 
             NpgsqlConnection C = new NpgsqlConnection(ConnectionString);
-            
+
             C.Notice += this.Notice;
 
-            if (connector != null) {
+            if (connector != null)
+            {
                 C.Open();
             }
 
@@ -482,14 +506,16 @@ namespace Npgsql
         //
         internal void OnNotice(object O, NpgsqlNoticeEventArgs E)
         {
-            if (Notice != null) {
+            if (Notice != null)
+            {
                 Notice(this, E);
             }
         }
 
         internal void OnNotification(object O, NpgsqlNotificationEventArgs E)
         {
-            if (Notification != null) {
+            if (Notification != null)
+            {
                 Notification(this, E);
             }
         }
@@ -542,9 +568,9 @@ namespace Npgsql
             get
             {
                 return (
-                    connection_string.ToBool(ConnectionStringKeys.Pooling, ConnectionStringDefaults.Pooling) &&
-                    connection_string.ToInt32(ConnectionStringKeys.MaxPoolSize, ConnectionStringDefaults.MaxPoolSize) > 0
-                );
+                           connection_string.ToBool(ConnectionStringKeys.Pooling, ConnectionStringDefaults.Pooling) &&
+                           connection_string.ToInt32(ConnectionStringKeys.MaxPoolSize, ConnectionStringDefaults.MaxPoolSize) > 0
+                       );
             }
         }
 
@@ -584,9 +610,12 @@ namespace Npgsql
             string                         targetHost,
             X509CertificateCollection      serverRequestedCertificates)
         {
-            if (CertificateSelectionCallback != null) {
+            if (CertificateSelectionCallback != null)
+            {
                 return CertificateSelectionCallback(clientCertificates, serverCertificate, targetHost, serverRequestedCertificates);
-            } else {
+            }
+            else
+            {
                 return null;
             }
         }
@@ -598,9 +627,12 @@ namespace Npgsql
             X509Certificate       certificate,
             int[]                 certificateErrors)
         {
-            if (CertificateValidationCallback != null) {
+            if (CertificateValidationCallback != null)
+            {
                 return CertificateValidationCallback(certificate, certificateErrors);
-            } else {
+            }
+            else
+            {
                 return true;
             }
         }
@@ -612,9 +644,12 @@ namespace Npgsql
             X509Certificate                certificate,
             string                         targetHost)
         {
-            if (PrivateKeySelectionCallback != null) {
+            if (PrivateKeySelectionCallback != null)
+            {
                 return PrivateKeySelectionCallback(certificate, targetHost);
-            } else {
+            }
+            else
+            {
                 return null;
             }
         }
@@ -624,43 +659,49 @@ namespace Npgsql
         //
         // Private methods and properties
         //
-       
+
 
         /// <summary>
         /// Write each key/value pair in the connection string to the log.
         /// </summary>
         private void LogConnectionString()
         {
-            foreach (DictionaryEntry DE in connection_string) {
+            foreach (DictionaryEntry DE in connection_string)
+            {
                 NpgsqlEventLog.LogMsg(resman, "Log_ConnectionStringValues", LogLevel.Debug, DE.Key, DE.Value);
             }
         }
 
         private void CheckConnectionOpen()
         {
-            if (disposed) {
+            if (disposed)
+            {
                 throw new ObjectDisposedException(CLASSNAME);
             }
 
-            if (connector == null) {
+            if (connector == null)
+            {
                 throw new InvalidOperationException(resman.GetString("Exception_ConnNotOpen"));
             }
         }
 
         private void CheckConnectionClosed()
         {
-            if (disposed) {
+            if (disposed)
+            {
                 throw new ObjectDisposedException(CLASSNAME);
             }
 
-            if (connector != null) {
+            if (connector != null)
+            {
                 throw new InvalidOperationException(resman.GetString("Exception_ConnOpen"));
             }
         }
 
         private void CheckNotDisposed()
         {
-            if (disposed) {
+            if (disposed)
+            {
                 throw new ObjectDisposedException(CLASSNAME);
             }
         }
index a076e578310beeb54be5cfae8066c6085328e343..95d7b751770745bbf8ff68aeadaa5dc011b1bfcd 100644 (file)
        <data name="Log_ConnectionStringValues">
                <value>ConnectionString Option: {0} = {1}</value>
        </data>
+       <data name="Log_ConnectionLeaking">
+               <value>NpgsqlConnection leaking! Fixing pool count...</value>
+       </data>
        <data name="Exception_NoNestedTransactions">
                <value>Nested/Concurrent transactions aren't supported.</value>
        </data>
index 1bf1d30b1b14c1f4eb08714f45db1f11e4b22037..0979f8c7000a885b3e637d4b6dffeea6e18cfd6b 100755 (executable)
@@ -90,7 +90,7 @@ namespace Npgsql
         private NpgsqlBackEndKeyData             _backend_keydata;
 
         // Flag for transaction status.
-//        private Boolean                         _inTransaction = false;
+        //        private Boolean                         _inTransaction = false;
         private NpgsqlTransaction                _transaction = null;
 
         private Boolean                          _supportsPrepare = false;
@@ -222,15 +222,38 @@ namespace Npgsql
 
 
 
-
         /// <summary>
-        /// Check for mediator errors (sent by backend) and throw the appropriate
-        /// exception if errors found.  This needs to be called after every interaction
-        /// with the backend.
+        /// This method checks if the connector is still ok.
+        /// We try to send a simple query text, select 1 as ConnectionTest;
         /// </summary>
-        internal void CheckErrors()
+        internal Boolean IsValid()
+        {
+            try
+            {
+                // Here we use a fake NpgsqlCommand, just to send the test query string.
+                Query(new NpgsqlCommand("select 1 as ConnectionTest"));
+            }
+            catch
+            {
+                return false;
+            }
+
+            return true;
+
+
+
+    }
+
+
+    /// <summary>
+    /// Check for mediator errors (sent by backend) and throw the appropriate
+    /// exception if errors found.  This needs to be called after every interaction
+    /// with the backend.
+    /// </summary>
+    internal void CheckErrors()
         {
-            if (_mediator.Errors.Count > 0) {
+            if (_mediator.Errors.Count > 0)
+            {
                 throw new NpgsqlException(_mediator.Errors);
             }
         }
@@ -242,8 +265,10 @@ namespace Npgsql
         /// </summary>
         internal void CheckNotices()
         {
-            if (Notice != null) {
-                foreach (NpgsqlError E in _mediator.Notices) {
+            if (Notice != null)
+            {
+                foreach (NpgsqlError E in _mediator.Notices)
+                {
                     Notice(this, new NpgsqlNoticeEventArgs(E));
                 }
             }
@@ -256,8 +281,10 @@ namespace Npgsql
         /// </summary>
         internal void CheckNotifications()
         {
-            if (Notification != null) {
-                foreach (NpgsqlNotificationEventArgs E in _mediator.Notifications) {
+            if (Notification != null)
+            {
+                foreach (NpgsqlNotificationEventArgs E in _mediator.Notifications)
+                {
                     Notification(this, E);
                 }
             }
@@ -282,9 +309,12 @@ namespace Npgsql
             string                         targetHost,
             X509CertificateCollection      serverRequestedCertificates)
         {
-            if (CertificateSelectionCallback != null) {
+            if (CertificateSelectionCallback != null)
+            {
                 return CertificateSelectionCallback(clientCertificates, serverCertificate, targetHost, serverRequestedCertificates);
-            } else {
+            }
+            else
+            {
                 return null;
             }
         }
@@ -296,9 +326,12 @@ namespace Npgsql
             X509Certificate       certificate,
             int[]                 certificateErrors)
         {
-            if (CertificateValidationCallback != null) {
+            if (CertificateValidationCallback != null)
+            {
                 return CertificateValidationCallback(certificate, certificateErrors);
-            } else {
+            }
+            else
+            {
                 return true;
             }
         }
@@ -310,9 +343,12 @@ namespace Npgsql
             X509Certificate                certificate,
             string                         targetHost)
         {
-            if (PrivateKeySelectionCallback != null) {
+            if (PrivateKeySelectionCallback != null)
+            {
                 return PrivateKeySelectionCallback(certificate, targetHost);
-            } else {
+            }
+            else
+            {
                 return null;
             }
         }
@@ -492,11 +528,14 @@ namespace Npgsql
         {
             ProtocolVersion      PV;
 
-            // If Connection.ConnectionString specifies a protocol version, we will 
+            // If Connection.ConnectionString specifies a protocol version, we will
             // not try to fall back to version 2 on failure.
-            if (ConnectionString.Contains(ConnectionStringKeys.Protocol)) {
+            if (ConnectionString.Contains(ConnectionStringKeys.Protocol))
+            {
                 PV = ConnectionString.ToProtocolVersion(ConnectionStringKeys.Protocol);
-            } else {
+            }
+            else
+            {
                 PV = ProtocolVersion.Unknown;
             }
 
@@ -516,7 +555,8 @@ namespace Npgsql
             if (_mediator.Errors.Count > 0 && PV == ProtocolVersion.Unknown)
             {
                 // If we attempted protocol version 3, it may be possible to drop back to version 2.
-                if (BackendProtocolVersion == ProtocolVersion.Version3) {
+                if (BackendProtocolVersion == ProtocolVersion.Version3)
+                {
                     NpgsqlError       Error0 = (NpgsqlError)_mediator.Errors[0];
 
                     // If NpgsqlError.ReadFromStream_Ver_3() encounters a version 2 error,
@@ -551,8 +591,8 @@ namespace Npgsql
 
             // First try to determine backend server version using the newest method.
             if (((NpgsqlParameterStatus)_mediator.Parameters["__npgsql_server_version"]) != null)
-                ServerVersionString = ((NpgsqlParameterStatus)_mediator.Parameters["__npgsql_server_version"]).ParameterValue; 
-            
+                ServerVersionString = ((NpgsqlParameterStatus)_mediator.Parameters["__npgsql_server_version"]).ParameterValue;
+
 
             // Fall back to the old way, SELECT VERSION().
             // This should not happen for protocol version 3+.
@@ -597,9 +637,11 @@ namespace Npgsql
         /// </summary>
         internal void Close()
         {
-            try {
+            try
+            {
                 this.CurrentState.Close(this);
-            } catch {}
+            }
+            catch {}
         }
-    }
+}
 }
index b73f5e00ea3e436572a717e5c556483bfc42e7be..fb29d4320419a0fe6f01bf7e0d3816604077b7e4 100755 (executable)
@@ -37,7 +37,7 @@ namespace Npgsql
         /// <summary>
         /// A queue with an extra Int32 for keeping track of busy connections.
         /// </summary>
-        private class ConnectorQueue : System.Collections.Queue
+    private class ConnectorQueue : System.Collections.Queue
         {
             /// <summary>
             /// The number of pooled Connectors that belong to this queue but
@@ -81,9 +81,12 @@ namespace Npgsql
         {
             NpgsqlConnector     Connector;
 
-            if (Connection.Pooling) {
+            if (Connection.Pooling)
+            {
                 Connector = RequestPooledConnector(Connection);
-            } else {
+            }
+            else
+            {
                 Connector = GetNonPooledConnector(Connection);
             }
 
@@ -116,10 +119,14 @@ namespace Npgsql
                 }
             }
 
-            if (Connector == null) {
-                if (Connection.Timeout > 0) {
+            if (Connector == null)
+            {
+                if (Connection.Timeout > 0)
+                {
                     throw new Exception("Timeout while getting a connection from pool.");
-                } else {
+                }
+                else
+                {
                     throw new Exception("Connection pool exceeds maximum size.");
                 }
             }
@@ -138,9 +145,12 @@ namespace Npgsql
             // If sharing were implemented, I suppose Shared would be set based
             // on some property on the Connection.
 
-            if (! Shared) {
+            if (! Shared)
+            {
                 Connector = GetPooledConnector(Connection);
-            } else {
+            }
+            else
+            {
                 // Connection sharing? What's that?
                 throw new NotImplementedException("Internal: Shared pooling not implemented");
             }
@@ -160,9 +170,12 @@ namespace Npgsql
         /// <param name="ForceClose">Force the connector to close, even if it is pooled.</param>
         public void ReleaseConnector (NpgsqlConnection Connection, NpgsqlConnector Connector)
         {
-            if (Connector.Pooled) {
+            if (Connector.Pooled)
+            {
                 ReleasePooledConnector(Connection, Connector);
-            } else {
+            }
+            else
+            {
                 UngetNonPooledConnector(Connection, Connector);
             }
         }
@@ -183,9 +196,12 @@ namespace Npgsql
         /// </summary>
         private void ReleasePooledConnectorInternal (NpgsqlConnection Connection, NpgsqlConnector Connector)
         {
-            if (! Connector.Shared) {
+            if (! Connector.Shared)
+            {
                 UngetPooledConnector(Connection, Connector);
-            } else {
+            }
+            else
+            {
                 // Connection sharing? What's that?
                 throw new NotImplementedException("Internal: Shared pooling not implemented");
             }
@@ -221,41 +237,69 @@ namespace Npgsql
             // Try to find a queue.
             Queue = (ConnectorQueue)PooledConnectors[Connection.ConnectionString.ToString()];
 
-            if (Queue == null) {
+            if (Queue == null)
+            {
                 Queue = new ConnectorQueue();
                 PooledConnectors[Connection.ConnectionString.ToString()] = Queue;
             }
 
-            if (Queue.Count > 0) {
+            if (Queue.Count > 0)
+            {
                 // Found a queue with connectors.  Grab the top one.
-                Connector = (NpgsqlConnector)Queue.Dequeue();
-                Queue.UseCount++;
 
-                // We should do some sort of check here to see if this connector is still OK?
-            } else if (Queue.Count + Queue.UseCount < Connection.MaxPoolSize) {
+                // Check if the connector is still valid.
+
+                while (true)
+                {
+                    Connector = (NpgsqlConnector)Queue.Dequeue();
+                    if (Connector.IsValid())
+                    {
+                        Queue.UseCount++;
+                        break;
+                    }
+
+                    Queue.UseCount--;
+
+                    if (Queue.Count <= 0)
+                        return GetPooledConnector(Connection);
+
+
+                }
+
+
+
+            }
+            else if (Queue.Count + Queue.UseCount < Connection.MaxPoolSize)
+            {
                 Connector = CreateConnector(Connection);
 
                 Connector.CertificateSelectionCallback += Connection.CertificateSelectionCallbackDelegate;
                 Connector.CertificateValidationCallback += Connection.CertificateValidationCallbackDelegate;
                 Connector.PrivateKeySelectionCallback += Connection.PrivateKeySelectionCallbackDelegate;
 
-                try {
+                try
+                {
                     Connector.Open();
-                } catch {
-                    try {
+                }
+                catch {
+                    try
+                    {
                         Connector.Close();
-                    } catch {}
+                        }
+                        catch {}
 
-                    throw;
-                }
-            
+                        throw;
+                    }
 
-                Queue.UseCount++;
-            }
 
-            // Meet the MinPoolSize requirement if needed.
-            if (Connection.MinPoolSize > 0) {
-                while (Queue.Count + Queue.UseCount < Connection.MinPoolSize) {
+                    Queue.UseCount++;
+        }
+
+        // Meet the MinPoolSize requirement if needed.
+        if (Connection.MinPoolSize > 0)
+            {
+                while (Queue.Count + Queue.UseCount < Connection.MinPoolSize)
+                {
                     NpgsqlConnector Spare = CreateConnector(Connection);
 
                     Spare.CertificateSelectionCallback += Connection.CertificateSelectionCallbackDelegate;
@@ -289,10 +333,33 @@ namespace Npgsql
         private NpgsqlConnector CreateConnector(NpgsqlConnection Connection)
         {
             return new NpgsqlConnector(
-                Connection.ConnectionStringValues.Clone(),
-                Connection.Pooling,
-                false
-            );
+                       Connection.ConnectionStringValues.Clone(),
+                       Connection.Pooling,
+                       false
+                   );
+        }
+
+
+        /// <summary>
+        /// This method is only called when NpgsqlConnection.Dispose(false) is called which means a
+        /// finalization. This also means, an NpgsqlConnection was leak. We clear pool count so that
+        /// client doesn't end running out of connections from pool. When the connection is finalized, its underlying
+        /// socket is closed.
+        /// </summary
+        public void FixPoolCountBecauseOfConnectionDisposeFalse(NpgsqlConnection Connection)
+        {
+            ConnectorQueue           Queue;
+
+            // Prevent multithread access to connection pool count.
+            lock(this)
+            {
+                // Try to find a queue.
+                Queue = (ConnectorQueue)PooledConnectors[Connection.ConnectionString.ToString()];
+
+                if (Queue != null)
+                    Queue.UseCount--;
+
+            }
         }
 
         /// <summary>
@@ -305,7 +372,8 @@ namespace Npgsql
             Connector.CertificateValidationCallback -= Connection.CertificateValidationCallbackDelegate;
             Connector.PrivateKeySelectionCallback -= Connection.PrivateKeySelectionCallbackDelegate;
 
-            if (Connector.Transaction != null) {
+            if (Connector.Transaction != null)
+            {
                 Connector.Transaction.Cancel();
             }
 
@@ -323,7 +391,8 @@ namespace Npgsql
             // Find the queue.
             Queue = (ConnectorQueue)PooledConnectors[Connector.ConnectionString.ToString()];
 
-            if (Queue == null) {
+            if (Queue == null)
+            {
                 throw new InvalidOperationException("Internal: No connector queue found for existing connector.");
             }
 
@@ -333,23 +402,32 @@ namespace Npgsql
 
             Queue.UseCount--;
 
-            if (! Connector.IsInitialized) {
-                if (Connector.Transaction != null) {
+            if (! Connector.IsInitialized)
+            {
+                if (Connector.Transaction != null)
+                {
                     Connector.Transaction.Cancel();
                 }
 
                 Connector.Close();
-            } else {
-                if (Connector.Transaction != null) {
-                    try {
+            }
+            else
+            {
+                if (Connector.Transaction != null)
+                {
+                    try
+                    {
                         Connector.Transaction.Rollback();
-                    } catch {
-                        Connector.Close();
+                    }
+                    catch {
+                        Connector.Close()
+                        ;
                     }
                 }
-            }
+        }
 
-            if (Connector.State == System.Data.ConnectionState.Open) {
+        if (Connector.State == System.Data.ConnectionState.Open)
+            {
                 Queue.Enqueue(Connector);
             }
         }