Don't use SIGUSR1 to tell dumpcap to exit, use SIGINT: SIGINT is traditionally

used for this purpose and using it also prevents the 2 signals the child gets:
	- the user's Ctrl-C (which is sent as a SIGINT to both *shark and its
	  child dumpcap)
	- the signal *shark generates to shut down the child

from colliding (and running 2 signal handlers in the child).

It might be possible for tshark to not send the signal at all when it gets
SIGINT, but it doesn't do any harm now.

Also, do not call g_log() within the signal handler: doing so can cause
aborts (if g_log is being called by the process when the signal comes, the
2nd entrance into g_log is detected as a recursion).

This fixes https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2767


git-svn-id: http://anonsvn.wireshark.org/wireshark/trunk@29881 f5534014-38df-0310-8fa8-9805f1628bb7

1 files changed, 17 insertions, 38 deletions

diff --git a/dumpcap.c b/dumpcap.c
index 91003b351d..d3206ee43d 100644
--- a/dumpcap.c
+++ b/dumpcap.c
@@ -136,7 +136,7 @@ static void capture_loop_stop(void);
  * is interrupted by a signal on UN*X, just go back and try again to
  * read again.
  *
- * On UN*X, we catch SIGUSR1 as a "stop capturing" signal, and, in
+ * On UN*X, we catch SIGINT as a "stop capturing" signal, and, in
  * the signal handler, set a flag to stop capturing; however, without
  * a guarantee of that sort, we can't guarantee that we'll stop capturing
  * if the read will be retried and won't time out if no packets arrive.
@@ -159,7 +159,7 @@ static void capture_loop_stop(void);
  * exit pcap_dispatch() with an indication that no packets have arrived,
  * and will break out of the capture loop at that point.
  *
- * On Windows, we can't send a SIGUSR1 to stop capturing, so none of this
+ * On Windows, we can't send a SIGINT to stop capturing, so none of this
  * applies in any case.
  *
  * XXX - the various BSDs appear to define BSD in <sys/param.h>; we don't
@@ -241,7 +241,7 @@ static const char please_report[] =
     "(This is not a crash; please do not report it as such.)";
 
 /*
- * This needs to be static, so that the SIGUSR1 handler can clear the "go"
+ * This needs to be static, so that the SIGINT handler can clear the "go"
  * flag.
  */
 static loop_data   global_ld;
@@ -507,7 +507,7 @@ print_statistics_loop(gboolean machine_readable)
 
 #ifdef _WIN32
 static BOOL WINAPI
-capture_cleanup(DWORD dwCtrlType)
+capture_cleanup_handler(DWORD dwCtrlType)
 {
     /* CTRL_C_EVENT is sort of like SIGINT, CTRL_BREAK_EVENT is unique to
        Windows, CTRL_CLOSE_EVENT is sort of like SIGHUP, CTRL_LOGOFF_EVENT
@@ -539,16 +539,16 @@ capture_cleanup(DWORD dwCtrlType)
 }
 #else
 static void
-capture_cleanup(int signum)
+capture_cleanup_handler(int signum _U_)
 {
     /* On UN*X, we cleanly shut down the capture on SIGINT, SIGHUP, and
        SIGTERM.  We assume that if the user wanted it to keep running
        after they logged out, they'd have nohupped it. */
 
-    g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_INFO,
-        "Console: Signal");
-    g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_DEBUG,
-        "Console: Signal, signal value: %u", signum);
+    /* Note: don't call g_log() in the signal handler: if we happened to be in
+     * g_log() in process context when the signal came in, g_log will detect
+     * the "recursion" and abort.
+     */
 
     capture_loop_stop();
 }
@@ -1981,13 +1981,6 @@ capture_loop_open_output(capture_options *capture_opts, int *save_file_fd,
 }
 
 
-static void
-capture_loop_stop_signal_handler(int signo _U_)
-{
-  g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_INFO, "Signal: Stop capture");
-  capture_loop_stop();
-}
-
 #ifdef _WIN32
 #define TIME_GET() GetTickCount()
 #else
@@ -1999,9 +1992,6 @@ capture_loop_stop_signal_handler(int signo _U_)
 static gboolean
 capture_loop_start(capture_options *capture_opts, gboolean *stats_known, struct pcap_stat *stats)
 {
-#ifndef _WIN32
-  struct sigaction act;
-#endif
   time_t      upd_time, cur_time;
   time_t      start_time;
   int         err_close;
@@ -2047,22 +2037,6 @@ capture_loop_start(capture_options *capture_opts, gboolean *stats_known, struct
   /* We haven't yet gotten the capture statistics. */
   *stats_known      = FALSE;
 
-#ifndef _WIN32
-  /*
-   * Catch SIGUSR1, so that we exit cleanly if the parent process
-   * kills us with it due to the user selecting "Capture->Stop".
-   */
-  act.sa_handler = capture_loop_stop_signal_handler;
-  /*
-   * Arrange that system calls not get restarted, because when
-   * our signal handler returns we don't want to restart
-   * a call that was waiting for packets to arrive.
-   */
-  act.sa_flags = 0;
-  sigemptyset(&act.sa_mask);
-  sigaction(SIGUSR1, &act, NULL);
-#endif /* _WIN32 */
-
   g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_INFO, "Capture loop starting ...");
   capture_opts_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_DEBUG, capture_opts);
 
@@ -2677,7 +2651,7 @@ main(int argc, char *argv[])
   WSAStartup( MAKEWORD( 1, 1 ), &wsaData );
 
   /* Set handler for Ctrl+C key */
-  SetConsoleCtrlHandler(capture_cleanup, TRUE);
+  SetConsoleCtrlHandler(capture_cleanup_handler, TRUE);
 
   /* Prepare to read from a pipe */
   if (!g_thread_supported ())
@@ -2689,7 +2663,12 @@ main(int argc, char *argv[])
 #else
   /* Catch SIGINT and SIGTERM and, if we get either of them, clean up
      and exit. */
-  action.sa_handler = capture_cleanup;
+  action.sa_handler = capture_cleanup_handler;
+  /*
+   * Arrange that system calls not get restarted, because when
+   * our signal handler returns we don't want to restart
+   * a call that was waiting for packets to arrive.
+   */
   action.sa_flags = 0;
   sigemptyset(&action.sa_mask);
   sigaction(SIGTERM, &action, NULL);
@@ -2756,7 +2735,7 @@ main(int argc, char *argv[])
   /*                                                                   */
   /*        It is therefore conceivable that if dumpcap somehow hangs  */
   /*        in pcap_open_live or before that wireshark will not        */
-  /*        be able to stop dumpcap using a signal (USR1, TERM, etc).  */
+  /*        be able to stop dumpcap using a signal (INT, TERM, etc).  */
   /*        In this case, exiting wireshark will kill the child        */
   /*        dumpcap process.                                           */
   /*                                                                   */