commit e917a278d836ad9cceba8ea5054346b22b3bbbb7
author Sebastian Zenker <sebastian.zenker@gmx.de> Mon Jan 18 08:45:52 2016 +0100
committer Nobuaki Sukegawa <nsuke@apache.org> Tue Jan 19 02:48:07 2016 +0900
tree 926fd16981c73d92b8f6c02a63ce51275af6d3a7
parent 3506b66cc04d3a9f6860b8cd7f4275468b91ac31

THRIFT-3564: potential core dump in TQTcpServer
Client: C++
Patch: Sebastian Zenker

lib/cpp/src/thrift/qt/TQTcpServer.cpp

diff --git a/lib/cpp/src/thrift/qt/TQTcpServer.cpp b/lib/cpp/src/thrift/qt/TQTcpServer.cpp
index a3211df..350198c 100644
--- a/lib/cpp/src/thrift/qt/TQTcpServer.cpp
+++ b/lib/cpp/src/thrift/qt/TQTcpServer.cpp
@@ -20,6 +20,7 @@
 #include <thrift/qt/TQTcpServer.h>
 #include <thrift/qt/TQIODeviceTransport.h>
 
+#include <QMetaType>
 #include <QTcpSocket>
 
 #include <thrift/cxxfunctional.h>
@@ -60,6 +61,7 @@
                          shared_ptr<TProtocolFactory> pfact,
                          QObject* parent)
   : QObject(parent), server_(server), processor_(processor), pfact_(pfact) {
+  qRegisterMetaType<QTcpSocket*>("QTcpSocket*");
   connect(server.get(), SIGNAL(newConnection()), SLOT(processIncoming()));
 }
 
@@ -91,8 +93,7 @@
 
     connect(connection.get(), SIGNAL(readyRead()), SLOT(beginDecode()));
 
-    // need to use QueuedConnection since we will be deleting the socket in the slot
-    connect(connection.get(), SIGNAL(disconnected()), SLOT(socketClosed()), Qt::QueuedConnection);
+    connect(connection.get(), SIGNAL(disconnected()), SLOT(socketClosed()));
   }
 }
 
@@ -114,29 +115,34 @@
                   ctx->oprot_);
   } catch (const TTransportException& ex) {
     qWarning("[TQTcpServer] TTransportException during processing: '%s'", ex.what());
-    ctxMap_.erase(connection);
+    scheduleDeleteConnectionContext(connection);
   } catch (...) {
     qWarning("[TQTcpServer] Unknown processor exception");
-    ctxMap_.erase(connection);
+    scheduleDeleteConnectionContext(connection);
   }
 }
 
 void TQTcpServer::socketClosed() {
   QTcpSocket* connection(qobject_cast<QTcpSocket*>(sender()));
   Q_ASSERT(connection);
+  scheduleDeleteConnectionContext(connection);
+}
 
-  if (ctxMap_.find(connection) == ctxMap_.end()) {
-    qWarning("[TQTcpServer] Unknown QTcpSocket closed");
-    return;
+void TQTcpServer::deleteConnectionContext(QTcpSocket* connection) {
+  const ConnectionContextMap::size_type deleted = ctxMap_.erase(connection);
+  if (0 == deleted) {
+      qWarning("[TQTcpServer] Unknown QTcpSocket");
   }
+}
 
-  ctxMap_.erase(connection);
+void TQTcpServer::scheduleDeleteConnectionContext(QTcpSocket* connection) {
+  QMetaObject::invokeMethod(this, "deleteConnectionContext", Qt::QueuedConnection, Q_ARG(QTcpSocket*, connection));
 }
 
 void TQTcpServer::finish(shared_ptr<ConnectionContext> ctx, bool healthy) {
   if (!healthy) {
     qWarning("[TQTcpServer] Processor failed to process data successfully");
-    ctxMap_.erase(ctx->connection_.get());
+    deleteConnectionContext(ctx->connection_.get());
   }
 }
 }

lib/cpp/src/thrift/qt/TQTcpServer.h

diff --git a/lib/cpp/src/thrift/qt/TQTcpServer.h b/lib/cpp/src/thrift/qt/TQTcpServer.h
index ebe6432..081d84a 100644
--- a/lib/cpp/src/thrift/qt/TQTcpServer.h
+++ b/lib/cpp/src/thrift/qt/TQTcpServer.h
@@ -57,19 +57,22 @@
   void processIncoming();
   void beginDecode();
   void socketClosed();
+  void deleteConnectionContext(QTcpSocket* connection);
 
 private:
   Q_DISABLE_COPY(TQTcpServer)
 
   struct ConnectionContext;
 
+  void scheduleDeleteConnectionContext(QTcpSocket* connection);
   void finish(boost::shared_ptr<ConnectionContext> ctx, bool healthy);
 
   boost::shared_ptr<QTcpServer> server_;
   boost::shared_ptr<TAsyncProcessor> processor_;
   boost::shared_ptr<apache::thrift::protocol::TProtocolFactory> pfact_;
 
-  std::map<QTcpSocket*, boost::shared_ptr<ConnectionContext> > ctxMap_;
+  typedef std::map<QTcpSocket*, boost::shared_ptr<ConnectionContext> > ConnectionContextMap;
+  ConnectionContextMap ctxMap_;
 };
 }
 }