[Commit-gnuradio] [gnuradio] 02/04: controlport: info on a (possible) Thrift bug.

[git]

This is an automated email from the git hooks/post-receive script.

jcorgan pushed a commit to branch master
in repository gnuradio.

commit 621c086b94e1f9b70f24034bf6fb6f7e15e5fa7c
Author: Tom Rondeau <address@hidden>
Date:   Thu Apr 16 10:37:30 2015 -0400

    controlport: info on a (possible) Thrift bug.
    
    A patch for Thrift itself and a README to provide some explanation.
---
 gnuradio-runtime/lib/controlport/thrift/README     | 41 +++++++++
 .../thrift/thrift-codebase-shutdown-patch.diff     | 98 ++++++++++++++++++++++
 2 files changed, 139 insertions(+)

diff --git a/gnuradio-runtime/lib/controlport/thrift/README 
b/gnuradio-runtime/lib/controlport/thrift/README
new file mode 100644
index 0000000..5448e51
--- /dev/null
+++ b/gnuradio-runtime/lib/controlport/thrift/README
@@ -0,0 +1,41 @@
+For info on ControlPort and Thrift, see the wiki page:
+
+http://gnuradio.org/redmine/projects/gnuradio/wiki/ControlPort
+
+This readme is to address the patch file in the repo for Thrift. We
+believe that there is a bug in Thrift itself that occasionally causes
+a segfault when shutting down the server. If you run into a problem
+when shutting down an application running ControlPort, this might be
+the cause. If so, we have included a patch with the source code in
+this directory that should be applied to Thrift before rebuilding the
+Thrift library:
+
+thrift-codebase-shutdown-patch.diff
+
+Note that we are working off Thrift version 0.9.2, so this patch may
+not be relevant or even cleanly apply in later versions of Thrift.
+
+We are also still trying to track down another shutdown problem with
+Thrift that again appears to be related to their code. If a segfault
+persists after the above patch is applied to Thrift, do a backtrace in
+GDB to see what the error looks like. If it is different than the
+following backtrace, definitely report it to the Discuss GNU Radio
+Mailing List as a known bug. If you see the same backtrace as here,
+this is a known bug that occurs very irregularly, but help in tracing
+down the cause is appreciated.
+
+#0  0x00007f07b5b478c2 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
+(gdb) bt
+#0  0x00007f07b5b478c2 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
+#1  0x00007f07b5b4856d in ?? () from /lib/x86_64-linux-gnu/libc.so.6
+#2  0x00007f07afc924c5 in std::locale::_Impl::~_Impl() () from 
/usr/lib/x86_64-linux-gnu/libstdc++.so.6
+#3  0x00007f07afc925fd in std::locale::~locale() () from 
/usr/lib/x86_64-linux-gnu/libstdc++.so.6
+#4  0x00007f07b5b05259 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
+#5  0x00007f07b5b052a5 in exit () from /lib/x86_64-linux-gnu/libc.so.6
+#6  0x000000000042fb83 in Py_Exit ()
+#7  0x000000000042fcb9 in ?? ()
+#8  0x000000000042ec32 in PyErr_PrintEx ()
+#9  0x0000000000469f2f in PyRun_SimpleFileExFlags ()
+#10 0x000000000046ab81 in Py_Main ()
+#11 0x00007f07b5aeaec5 in __libc_start_main () from 
/lib/x86_64-linux-gnu/libc.so.6
+#12 0x000000000057497e in _start ()
diff --git 
a/gnuradio-runtime/lib/controlport/thrift/thrift-codebase-shutdown-patch.diff 
b/gnuradio-runtime/lib/controlport/thrift/thrift-codebase-shutdown-patch.diff
new file mode 100644
index 0000000..eaf54d5
--- /dev/null
+++ 
b/gnuradio-runtime/lib/controlport/thrift/thrift-codebase-shutdown-patch.diff
@@ -0,0 +1,98 @@
+diff --git a/lib/cpp/src/thrift/concurrency/ThreadManager.cpp 
b/lib/cpp/src/thrift/concurrency/ThreadManager.cpp
+index 255d237..9954a8a 100644
+--- a/lib/cpp/src/thrift/concurrency/ThreadManager.cpp
++++ b/lib/cpp/src/thrift/concurrency/ThreadManager.cpp
+@@ -409,6 +409,8 @@ void ThreadManager::Impl::removeWorker(size_t value) {
+
+     workerMaxCount_ -= value;
+
++    shutdown_mutex_.unlock();
++
+     if (idleCount_ < value) {
+       for (size_t ix = 0; ix < idleCount_; ix++) {
+         monitor_.notify();
+diff --git a/lib/cpp/src/thrift/concurrency/ThreadManager.h 
b/lib/cpp/src/thrift/concurrency/ThreadManager.h
+index 7bb71d1..e97fd25 100644
+--- a/lib/cpp/src/thrift/concurrency/ThreadManager.h
++++ b/lib/cpp/src/thrift/concurrency/ThreadManager.h
+@@ -24,6 +24,7 @@
+ #include <thrift/cxxfunctional.h>
+ #include <sys/types.h>
+ #include <thrift/concurrency/Thread.h>
++#include <thrift/concurrency/Mutex.h>
+
+ namespace apache {
+ namespace thrift {
+@@ -59,6 +60,7 @@ protected:
+   ThreadManager() {}
+
+ public:
++  Mutex shutdown_mutex_;
+   typedef apache::thrift::stdcxx::function<void(boost::shared_ptr<Runnable>)> 
ExpireCallback;
+
+   virtual ~ThreadManager() {}
+diff --git a/lib/cpp/src/thrift/server/TThreadPoolServer.cpp 
b/lib/cpp/src/thrift/server/TThreadPoolServer.cpp
+index 0530d8d..d6b73dc 100644
+--- a/lib/cpp/src/thrift/server/TThreadPoolServer.cpp
++++ b/lib/cpp/src/thrift/server/TThreadPoolServer.cpp
+@@ -194,19 +194,28 @@ void TThreadPoolServer::serve() {
+     }
+   }
+
+-  // If stopped manually, join the existing threads
+-  if (stop_) {
+-    try {
+-      serverTransport_->close();
+-      threadManager_->join();
+-    } catch (TException& tx) {
+-      string errStr = string("TThreadPoolServer: Exception shutting down: ") 
+ tx.what();
+-      GlobalOutput(errStr.c_str());
++  {
++    Guard g(threadManager_->shutdown_mutex_);
++    // If stopped manually, join the existing threads
++    if (stop_) {
++      try {
++        serverTransport_->close();
++        threadManager_->join();
++      } catch (TException& tx) {
++        string errStr = string("TThreadPoolServer: Exception shutting down: 
") + tx.what();
++        GlobalOutput(errStr.c_str());
++      }
++      stop_ = false;
+     }
+-    stop_ = false;
+   }
+ }
+
++void TThreadPoolServer::stop() {
++    threadManager_->shutdown_mutex_.lock();
++    stop_ = true;
++    serverTransport_->interrupt();
++}
++
+ int64_t TThreadPoolServer::getTimeout() const {
+   return timeout_;
+ }
+diff --git a/lib/cpp/src/thrift/server/TThreadPoolServer.h 
b/lib/cpp/src/thrift/server/TThreadPoolServer.h
+index ad7e7ef..9b89846 100644
+--- a/lib/cpp/src/thrift/server/TThreadPoolServer.h
++++ b/lib/cpp/src/thrift/server/TThreadPoolServer.h
+@@ -24,6 +24,7 @@
+ #include <thrift/server/TServer.h>
+ #include <thrift/transport/TServerTransport.h>
+
++
+ #include <boost/shared_ptr.hpp>
+
+ namespace apache {
+@@ -113,10 +114,7 @@ public:
+
+   virtual void setTimeout(int64_t value);
+
+-  virtual void stop() {
+-    stop_ = true;
+-    serverTransport_->interrupt();
+-  }
++  virtual void stop();
+
+   virtual int64_t getTaskExpiration() const;