commit b8d11c5e07aa1dcc8e7ec4ffff645d0589579dea Author: David Smith 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 } +}