trial fix for ticket:137. Merged -r6196:6200 from eb/signal to trunk

git-svn-id: http://gnuradio.org/svn/gnuradio/trunk@6201 221aa14e-8319-0410-a670-987f0aec2ac5

5 files changed, 111 insertions, 34 deletions

diff --git a/gnuradio-core/src/lib/runtime/gr_hier_block2.h b/gnuradio-core/src/lib/runtime/gr_hier_block2.h
index ba25b4b87..51eb687ed 100644
--- a/gnuradio-core/src/lib/runtime/gr_hier_block2.h
+++ b/gnuradio-core/src/lib/runtime/gr_hier_block2.h
@@ -60,9 +60,30 @@ public:
   
   void connect(gr_basic_block_sptr src, int src_port, 
 	       gr_basic_block_sptr dst, int dst_port);
+
   void disconnect(gr_basic_block_sptr src, int src_port,
 		  gr_basic_block_sptr dst, int dst_port);
+
+  /*!
+   * Lock a flowgraph in preparation for reconfiguration.  When an equal
+   * number of calls to lock() and unlock() have occurred, the flowgraph
+   * will be restarted automatically.
+   *
+   * N.B. lock() and unlock() cannot be called from a flowgraph thread
+   * (E.g., gr_block::work method) or deadlock will occur when
+   * reconfiguration happens.
+   */
   virtual void lock();
+
+  /*!
+   * Lock a flowgraph in preparation for reconfiguration.  When an equal
+   * number of calls to lock() and unlock() have occurred, the flowgraph
+   * will be restarted automatically.
+   *
+   * N.B. lock() and unlock() cannot be called from a flowgraph thread
+   * (E.g., gr_block::work method) or deadlock will occur when
+   * reconfiguration happens.
+   */
   virtual void unlock();
 
   gr_flat_flowgraph_sptr flatten() const;
diff --git a/gnuradio-core/src/lib/runtime/gr_scheduler_thread.cc b/gnuradio-core/src/lib/runtime/gr_scheduler_thread.cc
index 16f36ac56..bc8f9b97e 100644
--- a/gnuradio-core/src/lib/runtime/gr_scheduler_thread.cc
+++ b/gnuradio-core/src/lib/runtime/gr_scheduler_thread.cc
@@ -54,15 +54,45 @@ void gr_scheduler_thread::start()
 void *
 gr_scheduler_thread::run_undetached(void *arg)
 {
-  // First code to run in new thread context
+  // This is the first code to run in the new thread context.
 
-  // Mask off SIGINT in this thread to gaurantee mainline thread gets signal
-#ifdef HAVE_SIGPROCMASK
+  /*
+   * In general, on a *nix system, any thread of a process can receive
+   * any asynchronous signal.
+   *
+   * http://www.serpentine.com/blog/threads-faq/mixing-threads-and-signals-unix/
+   * http://www.linuxjournal.com/article/2121
+   * 
+   * We really don't want to be handling asynchronous signals such
+   * as SIGINT and SIGHUP here.  We mask them off in the signal
+   * processing threads so that they'll get handled by the mainline
+   * thread.  We leave the synchronous signals SIGQUIT, SIGBUS,
+   * SIGILL, SIGSEGV etc alone
+   *
+   * FIXME? It might be better to mask them all off in the parent
+   * thread then dedicate a single thread to handling all signals
+   * using sigwait.
+   */
+#if defined(HAVE_PTHREAD_SIGMASK) || defined(HAVE_SIGPROCMASK)
   sigset_t old_set;
   sigset_t new_set;
+  int r;
   sigemptyset(&new_set);
   sigaddset(&new_set, SIGINT);
-  sigprocmask(SIG_BLOCK, &new_set, &old_set);
+  sigaddset(&new_set, SIGHUP);
+  sigaddset(&new_set, SIGPIPE);
+  sigaddset(&new_set, SIGALRM);
+  sigaddset(&new_set, SIGCHLD);
+
+#ifdef HAVE_PTHREAD_SIGMASK
+  r = pthread_sigmask(SIG_BLOCK, &new_set, &old_set);
+  if (r != 0)
+    perror("pthread_sigmask");
+#else
+  r = sigprocmask(SIG_BLOCK, &new_set, &old_set);
+  if (r != 0)
+    perror("sigprocmask");
+#endif
 #endif
   // Run the single-threaded scheduler
   d_sts->run();
@@ -72,7 +102,7 @@ gr_scheduler_thread::run_undetached(void *arg)
 void
 gr_scheduler_thread::stop()
 {
-  if (GR_SCHEDULER_THREAD_DEBUG)
+  if (0 && GR_SCHEDULER_THREAD_DEBUG)		// FIXME not safe to call from signal handler
     std::cout << "gr_scheduler_thread::stop() "
 	      << this << std::endl;
   d_sts->stop();
diff --git a/gnuradio-core/src/lib/runtime/gr_top_block.cc b/gnuradio-core/src/lib/runtime/gr_top_block.cc
index 407df8bba..91248d347 100644
--- a/gnuradio-core/src/lib/runtime/gr_top_block.cc
+++ b/gnuradio-core/src/lib/runtime/gr_top_block.cc
@@ -46,6 +46,9 @@ gr_top_block::gr_top_block(const std::string &name)
   
 gr_top_block::~gr_top_block()
 {
+  stop();
+  wait();
+    
   delete d_impl;
 }
 
diff --git a/gnuradio-core/src/lib/runtime/gr_top_block.h b/gnuradio-core/src/lib/runtime/gr_top_block.h
index 16fd97074..d74903841 100644
--- a/gnuradio-core/src/lib/runtime/gr_top_block.h
+++ b/gnuradio-core/src/lib/runtime/gr_top_block.h
@@ -47,37 +47,43 @@ public:
   ~gr_top_block();
 
   /*!
-   * Start the enclosed flowgraph.  Creates an undetached scheduler thread for
-   * each flow graph partition. Returns to caller once created.
+   * \brief The simple interface to running a flowgraph.
+   *
+   * Calls start() then wait().  Used to run a flowgraph that will stop
+   * on its own, or to run a flowgraph indefinitely until SIGINT is
+   * received.
+   */
+  void run();
+
+  /*!
+   * Start the contained flowgraph.  Creates one or more threads to
+   * execute the flow graph.  Returns to the caller once the threads
+   * are created.
    */
   void start();
   
   /*!
-   * Stop the running flowgraph.  Tells each created scheduler thread
-   * to exit, then returns to caller.
+   * Stop the running flowgraph.  Notifies each thread created by the
+   * scheduler to shutdown, then returns to caller.
    */
   void stop();
 
   /*!
-   * Wait for a stopped flowgraph to complete.  Joins each completed
-   * thread.
+   * Wait for a flowgraph to complete.  Flowgraphs complete when
+   * either (1) all blocks indicate that they are done (typically only
+   * when using gr.file_source, or gr.head, or (2) after stop() has been
+   * called to request shutdown.
    */
   void wait();
 
   /*!
-   * Calls start(), then wait().  Used to run a flowgraph that will stop
-   * on its own, or to run a flowgraph indefinitely until SIGKILL is
-   * received().
-   */
-  void run();
-
-  /*!
    * Lock a flowgraph in preparation for reconfiguration.  When an equal
    * number of calls to lock() and unlock() have occurred, the flowgraph
    * will be restarted automatically.
    *
-   * N.B. lock() and unlock() cannot be called from a flowgraph thread or
-   * deadlock will occur when reconfiguration happens.
+   * N.B. lock() and unlock() cannot be called from a flowgraph thread
+   * (E.g., gr_block::work method) or deadlock will occur when
+   * reconfiguration happens.
    */
   virtual void lock();
 
@@ -86,8 +92,9 @@ public:
    * number of calls to lock() and unlock() have occurred, the flowgraph
    * will be restarted automatically.
    *
-   * N.B. lock() and unlock() cannot be called from a flowgraph thread or
-   * deadlock will occur when reconfiguration happens.
+   * N.B. lock() and unlock() cannot be called from a flowgraph thread
+   * (E.g., gr_block::work method) or deadlock will occur when
+   * reconfiguration happens.
    */
   virtual void unlock();
 };
diff --git a/gnuradio-core/src/lib/runtime/gr_top_block_impl.cc b/gnuradio-core/src/lib/runtime/gr_top_block_impl.cc
index f25ab2b3e..340e81afd 100644
--- a/gnuradio-core/src/lib/runtime/gr_top_block_impl.cc
+++ b/gnuradio-core/src/lib/runtime/gr_top_block_impl.cc
@@ -32,15 +32,19 @@
 
 #include <stdexcept>
 #include <iostream>
+#include <string.h>
 
 #define GR_TOP_BLOCK_IMPL_DEBUG 0
 
 static gr_top_block_impl *s_impl = 0;
 
-// Make a vector of gr_block from a vector of gr_basic_block
-static
-gr_block_vector_t
-make_gr_block_vector(gr_basic_block_vector_t &blocks)
+/*!
+ * Make a vector of gr_block from a vector of gr_basic_block
+ *
+ * Pass-by-value to avoid problem with possible asynchronous modification
+ */
+static gr_block_vector_t
+make_gr_block_vector(gr_basic_block_vector_t blocks)
 {
   gr_block_vector_t result;
   for (gr_basic_block_viter_t p = blocks.begin(); p != blocks.end(); p++) {
@@ -51,22 +55,30 @@ make_gr_block_vector(gr_basic_block_vector_t &blocks)
 }
 
 // FIXME: This prevents using more than one gr_top_block instance
+
 static void 
 runtime_sigint_handler(int signum)
 {
-  if (GR_TOP_BLOCK_IMPL_DEBUG)
-    std::cout << "SIGINT received, calling stop()" << std::endl;
+  if (GR_TOP_BLOCK_IMPL_DEBUG){
+    char *msg = "SIGINT received, calling stop()\n";
+    ::write(1, msg, strlen(msg));	// write is OK to call from signal handler
+  }
 
   if (s_impl)
     s_impl->stop();
 }
 
+// ----------------------------------------------------------------
+
 gr_top_block_impl::gr_top_block_impl(gr_top_block *owner) 
   : d_running(false),
     d_ffg(),
     d_owner(owner),
     d_lock_count(0)
 {
+  if (s_impl)
+    throw std::logic_error("gr_top_block_impl: multiple simultaneous gr_top_block's");
+
   s_impl = this;
 }
 
@@ -115,18 +127,22 @@ gr_top_block_impl::start_threads()
   d_running = true;
 }
 
+/*
+ * N.B. as currently implemented, it is possible that this may be
+ * invoked by the SIGINT handler which is fragile as hell...
+ */
 void
 gr_top_block_impl::stop()
 {
-  if (GR_TOP_BLOCK_IMPL_DEBUG)
-    std::cout << "stop: entered" << std::endl;
+  if (GR_TOP_BLOCK_IMPL_DEBUG){
+    char *msg = "stop: entered\n";
+    ::write(1, msg, strlen(msg));
+  }
 
   for (gr_scheduler_thread_viter_t p = d_threads.begin(); p != d_threads.end(); p++) {
-    if (GR_TOP_BLOCK_IMPL_DEBUG)
-      std::cout << "stop: stopping thread " << (*p) << std::endl;
-    (*p)->stop();
+    if (*p)
+      (*p)->stop();
   }
-
   d_running = false;
 }