Remove unnecessary locking from QNetworkProxy constructor
authorShane Kearns <ext-shane.2.kearns@nokia.com>
Thu, 16 Feb 2012 17:02:11 +0000 (17:02 +0000)
committerQt by Nokia <qt-info@nokia.com>
Fri, 17 Feb 2012 14:04:22 +0000 (15:04 +0100)
QGlobalNetworkProxy (a singleton) had two phase construction, with
the second phase being called from QNetworkProxy's constructor.
This isn't necessary, and has been reported as causing deadlocks.

Although constructing socket engine handlers has side effects
(they add themselves to a list on construction and remove themselves
on destruction), this appears to be safe. The socket engine handlers
are only used while holding the list mutex, and any socket engines
created don't have any reference to the factory that created them.

With the new version, it is possible that two instances of
QHttpSocketEngineHandler and QSocks5SocketEngineHandler exist
temporarily if a Q_GLOBAL_STATIC initialisation race occurs.
This appears safe, because the loser of the race deletes its
handlers, which remove themselves from the global list as above.

Task-number: QTBUG-13088
Change-Id: I8cf520da717d8ab7d862ab89c6de13aea6d60ac3
Reviewed-by: Richard J. Moore <rich@kde.org>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
src/network/kernel/qnetworkproxy.cpp

index 1181497..0238d22 100644 (file)
@@ -247,6 +247,12 @@ public:
         , socks5SocketEngineHandler(0)
         , httpSocketEngineHandler(0)
     {
+#ifndef QT_NO_SOCKS5
+        socks5SocketEngineHandler = new QSocks5SocketEngineHandler();
+#endif
+#ifndef QT_NO_HTTP
+        httpSocketEngineHandler = new QHttpSocketEngineHandler();
+#endif
     }
 
     ~QGlobalNetworkProxy()
@@ -257,19 +263,6 @@ public:
         delete httpSocketEngineHandler;
     }
 
-    void init()
-    {
-        QMutexLocker lock(&mutex);
-#ifndef QT_NO_SOCKS5
-        if (!socks5SocketEngineHandler)
-            socks5SocketEngineHandler = new QSocks5SocketEngineHandler();
-#endif
-#ifndef QT_NO_HTTP
-        if (!httpSocketEngineHandler)
-            httpSocketEngineHandler = new QHttpSocketEngineHandler();
-#endif
-    }
-
     void setApplicationProxy(const QNetworkProxy &proxy)
     {
         QMutexLocker lock(&mutex);
@@ -431,8 +424,6 @@ template<> void QSharedDataPointer<QNetworkProxyPrivate>::detach()
 QNetworkProxy::QNetworkProxy()
     : d(0)
 {
-    if (QGlobalNetworkProxy *globalProxy = globalNetworkProxy())
-        globalProxy->init();
 }
 
 /*!
@@ -447,8 +438,6 @@ QNetworkProxy::QNetworkProxy(ProxyType type, const QString &hostName, quint16 po
                   const QString &user, const QString &password)
     : d(new QNetworkProxyPrivate(type, hostName, port, user, password))
 {
-    if (QGlobalNetworkProxy *globalProxy = globalNetworkProxy())
-        globalProxy->init();
 }
 
 /*!