Fix RConnection handle leak in symbian bearer plugin

The implementation was opening RConnection handles on top of previous
instances, and not closing RConnection handles. Both of these cause a
resource leak in the socket server which cannot clean up the connection
until the Qt process has exited.
After a lot of this (which could be triggered by the QNetworkReply auto
test), the socket server may run out of memory resulting in all socket
operations failing.

Reviewed-by: Markus Goetz
(cherry picked from commit f5e7b6c64caa67bf11fa9754114d686da73b22d6)

2 files changed, 41 insertions, 57 deletions

diff --git a/src/plugins/bearer/symbian/qnetworksession_impl.cpp b/src/plugins/bearer/symbian/qnetworksession_impl.cpp
index 5639c4f..fc6e141 100644
--- a/src/plugins/bearer/symbian/qnetworksession_impl.cpp
+++ b/src/plugins/bearer/symbian/qnetworksession_impl.cpp
@@ -89,33 +89,43 @@ QNetworkSessionPrivateImpl::QNetworkSessionPrivateImpl(SymbianEngine *engine)
     TRAP_IGNORE(iConnectionMonitor.ConnectL());
 }
 
-QNetworkSessionPrivateImpl::~QNetworkSessionPrivateImpl()
+void QNetworkSessionPrivateImpl::closeHandles()
 {
-    isOpen = false;
-    isOpening = false;
-
     // Cancel Connection Progress Notifications first.
-    // Note: ConnectionNotifier must be destroyed before Canceling RConnection::Start()
+    // Note: ConnectionNotifier must be destroyed before RConnection::Close()
     //       => deleting ipConnectionNotifier results RConnection::CancelProgressNotification()
     delete ipConnectionNotifier;
     ipConnectionNotifier = NULL;
 
 #ifdef SNAP_FUNCTIONALITY_AVAILABLE
-    if (iMobility) {
-        delete iMobility;
-        iMobility = NULL;
-    }
+    // mobility monitor must be deleted before RConnection is closed
+    delete iMobility;
+    iMobility = NULL;
 #endif
 
-    // Cancel possible RConnection::Start()
+    // Cancel possible RConnection::Start() - may call RConnection::Close if Start was in progress
     Cancel();
-    iSocketServ.Close();
+    //close any open connection (note Close twice is safe in case Cancel did it above)
+    iConnection.Close();
 
     // Close global 'Open C' RConnection
     // Clears also possible unsetdefaultif() flags.
     setdefaultif(0);
 
     iConnectionMonitor.Close();
+#ifdef QT_BEARERMGMT_SYMBIAN_DEBUG
+    qDebug() << "QNS this : " << QString::number((uint)this)
+             << " - handles closed";
+#endif
+    iSocketServ.Close();
+}
+
+QNetworkSessionPrivateImpl::~QNetworkSessionPrivateImpl()
+{
+    isOpen = false;
+    isOpening = false;
+
+    closeHandles();
     iOpenCLibrary.Close();
 #ifdef QT_BEARERMGMT_SYMBIAN_DEBUG
     qDebug() << "QNS this : " << QString::number((uint)this)
@@ -166,7 +176,7 @@ void QNetworkSessionPrivateImpl::configurationAdded(QNetworkConfigurationPrivate
 #ifdef QT_BEARERMGMT_SYMBIAN_DEBUG
         qDebug() << "QNS this : " << QString::number((uint)this) << " - "
                  << "configurationAdded IAP: "
-                 << toSymbianConfig(privateConfiguration(config))->numericIdentifier();
+                 << toSymbianConfig(config)->numericIdentifier();
 #endif
 
         syncStateWithInterface();
@@ -372,7 +382,8 @@ void QNetworkSessionPrivateImpl::open()
         syncStateWithInterface();    
         return;
     }
-    
+
+    Q_ASSERT(!iConnection.SubSessionHandle());
     error = iConnection.Open(iSocketServ);
     if (error != KErrNone) {
         // Could not open RConnection
@@ -414,8 +425,8 @@ void QNetworkSessionPrivateImpl::open()
 
             pref.SetIapId(symbianConfig->numericIdentifier());
 #endif
-            iConnection.Start(pref, iStatus);
             if (!IsActive()) {
+                iConnection.Start(pref, iStatus);
                 SetActive();
             }
             // Avoid flip flop of states if the configuration is already
@@ -442,8 +453,8 @@ void QNetworkSessionPrivateImpl::open()
 #else
         TConnSnapPref snapPref(symbianConfig->numericIdentifier());
 #endif
-        iConnection.Start(snapPref, iStatus);
         if (!IsActive()) {
+            iConnection.Start(snapPref, iStatus);
             SetActive();
         }
         // Avoid flip flop of states if the configuration is already
@@ -453,8 +464,8 @@ void QNetworkSessionPrivateImpl::open()
         }
     } else if (publicConfig.type() == QNetworkConfiguration::UserChoice) {
         iKnownConfigsBeforeConnectionStart = engine->accessPointConfigurationIdentifiers();
-        iConnection.Start(iStatus);
         if (!IsActive()) {
+            iConnection.Start(iStatus);
             SetActive();
         }
         newState(QNetworkSession::Connecting);
@@ -465,9 +476,7 @@ void QNetworkSessionPrivateImpl::open()
         isOpening = false;
         iError = QNetworkSession::UnknownSessionError;
         emit QNetworkSessionPrivate::error(iError);
-        if (ipConnectionNotifier) {
-            ipConnectionNotifier->StopNotifications();
-        }
+        closeHandles();
         syncStateWithInterface();    
     }
 }
@@ -521,21 +530,10 @@ void QNetworkSessionPrivateImpl::close(bool allowSignals)
 
     serviceConfig = QNetworkConfiguration();
     
-#ifdef SNAP_FUNCTIONALITY_AVAILABLE
-    if (iMobility) {
-        delete iMobility;
-        iMobility = NULL;
-    }
-#endif
+    closeHandles();
 
-    if (ipConnectionNotifier && !iHandleStateNotificationsFromManager) {
-        ipConnectionNotifier->StopNotifications();
-        // Start handling IAP state change signals from QNetworkConfigurationManagerPrivate
-        iHandleStateNotificationsFromManager = true;
-    }
-    
-    Cancel(); // closes iConnection
-    iSocketServ.Close();
+    // Start handling IAP state change signals from QNetworkConfigurationManagerPrivate
+    iHandleStateNotificationsFromManager = true;
 
     // Close global 'Open C' RConnection. If OpenC supports,
     // close the defaultif for good to avoid difficult timing
@@ -759,12 +757,14 @@ void QNetworkSessionPrivateImpl::NewCarrierActive(TAccessPointInfo /*aNewAPInfo*
     }
 }
 
-void QNetworkSessionPrivateImpl::Error(TInt /*aError*/)
+void QNetworkSessionPrivateImpl::Error(TInt aError)
 {
 #ifdef QT_BEARERMGMT_SYMBIAN_DEBUG
     qDebug() << "QNS this : " << QString::number((uint)this) << " - "
-            << "roaming Error() occurred, isOpen is: " << isOpen;
+            << "roaming Error() occurred" << aError << ", isOpen is: " << isOpen;
 #endif
+    if (aError == KErrCancel)
+        return; //avoid recursive deletion
     if (isOpen) {
         isOpen = false;
         isOpening = false;
@@ -772,10 +772,7 @@ void QNetworkSessionPrivateImpl::Error(TInt /*aError*/)
         serviceConfig = QNetworkConfiguration();
         iError = QNetworkSession::RoamingError;
         emit QNetworkSessionPrivate::error(iError);
-        Cancel();
-        if (ipConnectionNotifier) {
-            ipConnectionNotifier->StopNotifications();
-        }
+        closeHandles();
         QT_TRY {
             syncStateWithInterface();
             // In some cases IAP is still in Connected state when
@@ -1102,18 +1099,13 @@ void QNetworkSessionPrivateImpl::RunL()
                 isOpening = false;
                 iError = QNetworkSession::UnknownSessionError;
                 QT_TRYCATCH_LEAVING(emit QNetworkSessionPrivate::error(iError));
-                if (ipConnectionNotifier) {
-                    ipConnectionNotifier->StopNotifications();
-                }
+                closeHandles();
                 if (!newActiveConfig.isValid()) {
                     // No valid configuration, bail out.
                     // Status updates from QNCM won't be received correctly
                     // because there is no configuration to associate them with so transit here.
-                    iConnection.Close();
                     newState(QNetworkSession::Closing);
                     newState(QNetworkSession::Disconnected);
-                } else {
-                    Cancel();
                 }
                 QT_TRYCATCH_LEAVING(syncStateWithInterface());
                 return;
@@ -1156,10 +1148,7 @@ void QNetworkSessionPrivateImpl::RunL()
             serviceConfig = QNetworkConfiguration();
             iError = QNetworkSession::InvalidConfigurationError;
             QT_TRYCATCH_LEAVING(emit QNetworkSessionPrivate::error(iError));
-            if (ipConnectionNotifier) {
-                ipConnectionNotifier->StopNotifications();
-            }
-            Cancel();
+            closeHandles();
             QT_TRYCATCH_LEAVING(syncStateWithInterface());
             break;
         case KErrCancel: // Connection attempt cancelled
@@ -1178,10 +1167,7 @@ void QNetworkSessionPrivateImpl::RunL()
                 iError = QNetworkSession::UnknownSessionError;
             }
             QT_TRYCATCH_LEAVING(emit QNetworkSessionPrivate::error(iError));
-            if (ipConnectionNotifier) {
-                ipConnectionNotifier->StopNotifications();
-            }
-            Cancel();
+            closeHandles();
             QT_TRYCATCH_LEAVING(syncStateWithInterface());
             break;
     }
@@ -1273,10 +1259,7 @@ bool QNetworkSessionPrivateImpl::newState(QNetworkSession::State newState, TUint
         serviceConfig = QNetworkConfiguration();
         iError = QNetworkSession::SessionAbortedError;
         emit QNetworkSessionPrivate::error(iError);
-        if (ipConnectionNotifier) {
-            ipConnectionNotifier->StopNotifications();
-        }
-        Cancel();
+        closeHandles();
         // Start handling IAP state change signals from QNetworkConfigurationManagerPrivate
         iHandleStateNotificationsFromManager = true;
         emitSessionClosed = true; // Emit SessionClosed after state change has been reported
diff --git a/src/plugins/bearer/symbian/qnetworksession_impl.h b/src/plugins/bearer/symbian/qnetworksession_impl.h
index 774b988..a597812 100644
--- a/src/plugins/bearer/symbian/qnetworksession_impl.h
+++ b/src/plugins/bearer/symbian/qnetworksession_impl.h
@@ -149,6 +149,7 @@ private:
     bool easyWlanTrueIapId(TUint32 &trueIapId) const;
 #endif
 
+    void closeHandles();
 
 private: // data
     SymbianEngine *engine;