Blob Blame History Raw
commit b8d11c5e07aa1dcc8e7ec4ffff645d0589579dea
Author: David Smith <dsmith@redhat.com>
Date:   Fri Jan 26 09:24:51 2018 -0600

    BZ1527809 - Fix detaching from modules using SIGQUIT.
    
    * staprun/mainloop.c: Put a "/* NOTREACHED */" comment after all calls to
      cleanup_and_exit() to remind the reader that cleanup_and_exit() doesn't
      return.
      (stp_main_loop): If we've got a pending interrupt,
      but 'load_only' is set, just detach instead of sending STP_EXIT to
      the module. Otherwise using SIGQUIT to detach fails and unloads the
      module.
    * staprun/monitor.c: Put a "/* NOTREACHED */" comment after all calls to
      cleanup_and_exit() to remind the reader that cleanup_and_exit() doesn't
      return.
    * testsuite/systemtap.base/attach_detach.exp: New test.

diff --git a/staprun/mainloop.c b/staprun/mainloop.c
index a60372e..63b72cc 100644
--- a/staprun/mainloop.c
+++ b/staprun/mainloop.c
@@ -648,6 +648,7 @@ int stp_main_loop(void)
   if (rc != 0) {
     perror ("Unable to send STP_READY");
     cleanup_and_exit(0, rc);
+    /* NOTREACHED */
   }
 
   flags = fcntl(control_channel, F_GETFL);
@@ -695,12 +696,28 @@ int stp_main_loop(void)
 
     if (pending_interrupts) {
          int btype = STP_EXIT;
-         int rc = write(control_channel, &btype, sizeof(btype));
+         int rc;
+
+	 /* If 'load_only' is set, we don't want to send STP_EXIT,
+	  * which would cause any 'probe end' processing to be
+	  * done. Instead, we'll just detach by calling
+	  * cleanup_and_exit(). This should let the module continue to
+	  * run. */
+	 if (load_only)
+	   {
+	     cleanup_and_exit(load_only /* = detach */, 0);
+	     /* NOTREACHED */
+	   }
+
+         rc = write(control_channel, &btype, sizeof(btype));
          dbug(2, "signal-triggered %d exit rc %d\n", pending_interrupts, rc);
-         if (monitor || (pending_interrupts > 2)) /* user mashing on ^C multiple times */
-                 cleanup_and_exit (load_only /* = detach */, 0);
+         if (monitor || (pending_interrupts > 2))
+	   { /* user mashing on ^C multiple times */
+	     cleanup_and_exit (load_only /* = detach */, 0);
+	     /* NOTREACHED */
+	   }
          else
-                 {} /* await STP_EXIT reply message to kill staprun */
+           {} /* await STP_EXIT reply message to kill staprun */
     }
 
     /* If the runtime does not implement select() on the command
@@ -719,6 +736,7 @@ int stp_main_loop(void)
       if (nb >= 0 || (errno != EINTR && errno != EAGAIN)) {
         _perr(_("Unexpected EOF in read (nb=%ld)"), (long)nb);
         cleanup_and_exit(0, 1);
+	/* NOTREACHED */
       }
 
       if (!select_supported) {
@@ -736,6 +754,7 @@ int stp_main_loop(void)
 	  {
 	    _perr(_("Unexpected error in select"));
 	    cleanup_and_exit(0, 1);
+	    /* NOTREACHED */
 	  }
       }
       continue;
@@ -750,6 +769,7 @@ int stp_main_loop(void)
       if (write_realtime_data(recvbuf.payload.data, nb)) {
         _perr(_("write error (nb=%ld)"), (long)nb);
         cleanup_and_exit(0, 1);
+	/* NOTREACHED */
       }
       break;
 #endif
@@ -841,8 +861,10 @@ int stp_main_loop(void)
         dbug(2, "got STP_EXIT\n");
         if (monitor)
                 monitor_exited();
-        else
+        else {
                 cleanup_and_exit(0, error_detected);
+		/* NOTREACHED */
+	}
         /* monitor mode exit handled elsewhere, later. */
         break;
       }
@@ -863,6 +885,7 @@ int stp_main_loop(void)
           if (target_cmd)
             kill(target_pid, SIGKILL);
           cleanup_and_exit(0, 1);
+	  /* NOTREACHED */
         } else if (target_cmd) {
           dbug(1, "detaching pid %d\n", target_pid);
 #if WORKAROUND_BZ467568
@@ -878,6 +901,7 @@ int stp_main_loop(void)
               if (target_cmd)
                 kill(target_pid, SIGKILL);
               cleanup_and_exit(0, 1);
+	      /* NOTREACHED */
             }
 #endif
         }
@@ -901,20 +925,24 @@ int stp_main_loop(void)
         struct _stp_msg_start ts;
         struct _stp_msg_ns_pid nspid;
         if (use_old_transport) {
-          if (init_oldrelayfs() < 0)
+	  if (init_oldrelayfs() < 0) {
             cleanup_and_exit(0, 1);
+	    /* NOTREACHED */
+	  }
         } else {
           if (init_relayfs() < 0)
             cleanup_and_exit(0, 1);
+	    /* NOTREACHED */
         }
 
         if (target_namespaces_pid > 0) {
           nspid.target = target_namespaces_pid;
           rc = send_request(STP_NAMESPACES_PID, &nspid, sizeof(nspid));
           if (rc != 0) {
-	          perror ("Unable to send STP_NAMESPACES_PID");
-	          cleanup_and_exit (1, rc);
-	        }
+	    perror ("Unable to send STP_NAMESPACES_PID");
+	    cleanup_and_exit (1, rc);
+	    /* NOTREACHED */
+	  }
         }
 
         ts.target = target_pid;
@@ -922,9 +950,12 @@ int stp_main_loop(void)
 	if (rc != 0) {
 	  perror ("Unable to send STP_START");
 	  cleanup_and_exit(0, rc);
+	  /* NOTREACHED */
 	}
-        if (load_only)
+        if (load_only) {
           cleanup_and_exit(1, 0);
+	  /* NOTREACHED */
+	}
         break;
       }
     default:
diff --git a/staprun/monitor.c b/staprun/monitor.c
index 6b8bb11..478634c 100644
--- a/staprun/monitor.c
+++ b/staprun/monitor.c
@@ -598,7 +598,10 @@ void monitor_input(void)
               break;
             case 'q':
               if (monitor_state == exited)
-                cleanup_and_exit(0, 0 /* error_detected unavailable here */ );
+	        {
+		  cleanup_and_exit(0, 0 /* error_detected unavailable here */ );
+		  /* NOTREACHED */
+		}
               else
                 write_command("quit");
               break;
diff --git a/testsuite/systemtap.base/attach_detach.exp b/testsuite/systemtap.base/attach_detach.exp
new file mode 100644
index 0000000..ef23615
--- /dev/null
+++ b/testsuite/systemtap.base/attach_detach.exp
@@ -0,0 +1,145 @@
+set test "attach_detach"
+if {![installtest_p]} { untested $test; return }
+
+set test_script { "
+    probe begin { printf(\"begin probe fired\\n\") }
+    probe timer.s(5) { printf(\"timer probe fired\\n\") }
+    probe end { printf(\"end probe fired\\n\") }
+" }
+
+# First, compile a module.
+stap_compile $test 1 $test_script -m attach_detach
+
+# stap_compile does pass/fail, but doesn't return a status. So, if
+# attach_detach.ko exists, it worked.
+if {! [file exists attach_detach.ko]} {
+    return
+}
+
+# Load the module and detach.
+set subtest "initial load"
+spawn staprun -L attach_detach.ko
+set fail 0
+set pass 0
+expect {
+    -timeout 120
+    -re "^begin probe fired\r\n" { incr fail; exp_continue }
+    -re "^\r\n" { exp_continue }
+    -re "^Disconnecting from systemtap module.\r\n" {
+	incr pass; exp_continue
+    }
+    -re "^To reconnect, type \"staprun -A attach_detach\"\r\n" {
+	incr pass
+    }
+    eof { fail "$test ($subtest) - EOF"; incr fail }
+    timeout { fail "$test ($subtest) - unexpected timeout"; incr fail }
+}
+catch {close}; catch {wait}
+
+if {$fail == 0 && $pass == 2} {
+    pass "$test ($subtest) - disconnect seen"
+} else {
+    fail "$test ($subtest) - begin seen ($fail $pass)"
+}
+
+# Make sure the module is still loaded.
+if {[catch { exec lsmod | grep attach_detach >/dev/null }]} {
+    fail "$test ($subtest) - module still present"
+    return
+}
+pass "$test ($subtest) - module still present"
+
+# Attach to the module, then use SIGQUIT to detach again.
+set subtest "attach and SIGQUIT"
+spawn staprun -A attach_detach
+set fail 0
+set pass 0
+set timer_probe_seen 0
+expect {
+    -timeout 120
+    -re "^begin probe fired\r\n" { incr pass; exp_continue }
+    -re "^end probe fired\r\n" { incr fail; exp_continue }
+    -re "^timer probe fired\r\n" {
+	if {!$timer_probe_seen} {
+	    set timer_probe_seen 1
+	    incr pass
+
+	    # Send our staprun process a SIGQUIT, to make it detach.
+	    kill SIGQUIT [exp_pid]
+	}
+	exp_continue
+    }
+    -re "^\r\n" { exp_continue }
+    -re "^Disconnecting from systemtap module.\r\n" {
+	incr pass; exp_continue
+    }
+    -re "^To reconnect, type \"staprun -A attach_detach\"\r\n" {
+	incr pass
+    }
+    eof { fail "$test ($subtest) - EOF"; incr fail }
+    timeout { fail "$test ($subtest) - unexpected timeout"; incr fail }
+}    
+catch {close}; catch {wait}
+
+if {$fail == 0 && $pass == 4} {
+    pass "$test ($subtest) - disconnect seen"
+} else {
+    fail "$test ($subtest) - no disconnect seen ($fail $pass)"
+}
+
+# Make sure the module is still loaded.
+if {[catch { exec lsmod | grep attach_detach >/dev/null}]} {
+    fail "$test ($subtest) - module still present"
+    return
+}
+pass "$test ($subtest) - module still present"
+
+# Attach one last time, then use SIGTERM to unload the module and quit.
+set subtest "attach and SIGTERM"
+spawn staprun -A attach_detach
+set fail 0
+set pass 0
+set timer_probe_seen 0
+expect {
+    -timeout 120
+    -re "^begin probe fired\r\n" { incr fail; exp_continue }
+    -re "^end probe fired\r\n" { incr pass }
+    -re "^timer probe fired\r\n" {
+	if {!$timer_probe_seen} {
+	    set timer_probe_seen 1
+	    incr pass
+
+	    # Send our staprun process a SIGTERM, to make it quit and
+	    # unload.
+	    kill SIGTERM [exp_pid]
+	}
+	exp_continue
+    }
+    -re "^\r\n" { exp_continue }
+    -re "^Disconnecting from systemtap module.\r\n" {
+	incr fail; exp_continue
+    }
+    -re "^To reconnect, type \"staprun -A attach_detach\"\r\n" {
+	incr fail; exp_continue
+    }
+    eof { fail "$test ($subtest) - EOF"; incr fail }
+    timeout { fail "$test ($subtest) - unexpected timeout"; incr fail }
+}    
+catch {close}; catch {wait}
+
+if {$fail == 0 && $pass == 2} {
+    pass "$test ($subtest) - quit seen"
+} else {
+    fail "$test ($subtest) - no quit seen ($fail $pass)"
+}
+
+# Make sure the module isn't still loaded.
+if {[catch { exec lsmod | grep attach_detach >/dev/null}]} {
+    pass "$test ($subtest) - module is gone"
+} else {
+    fail "$test ($subtest) - module is gone"
+
+    # If for some odd reason the module is still loaded, try to unload
+    # it.
+    catch { exec staprun -d attach_detach }
+}