From owner-freebsd-arch@FreeBSD.ORG Sun Jan 2 22:31:21 2011 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0DBA4106566B for ; Sun, 2 Jan 2011 22:31:21 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (relay02.stack.nl [IPv6:2001:610:1108:5010::104]) by mx1.freebsd.org (Postfix) with ESMTP id 89BDE8FC14 for ; Sun, 2 Jan 2011 22:31:20 +0000 (UTC) Received: from turtle.stack.nl (turtle.stack.nl [IPv6:2001:610:1108:5010::132]) by mx1.stack.nl (Postfix) with ESMTP id 02931359956 for ; Sun, 2 Jan 2011 23:31:19 +0100 (CET) Received: by turtle.stack.nl (Postfix, from userid 1677) id EDD9A1727D; Sun, 2 Jan 2011 23:31:18 +0100 (CET) Date: Sun, 2 Jan 2011 23:31:18 +0100 From: Jilles Tjoelker To: freebsd-arch@freebsd.org Message-ID: <20110102223118.GA95551@stack.nl> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="CE+1k2dSO48ffgeK" Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Subject: init(8) running rc.shutdown inappropriately, mechanism for halt(8)/reboot(8) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 02 Jan 2011 22:31:21 -0000 --CE+1k2dSO48ffgeK Content-Type: text/plain; charset=us-ascii Content-Disposition: inline As discussed on the svn-src commits mailing list, I think halt and reboot should be changed to trigger the shutdown by sending a signal to init, either directly or by calling shutdown. The new poweroff utility should do the same as halt -p. A new -f option and the filenames fastboot and fasthalt keep the current behaviour of killing all processes and calling reboot(2) directly, and the same applies if one of the options -d, -n and -q is used (which cannot be passed to init). I'm aware of shutdown's -o option which makes it call halt(8) or reboot(8). These invocations should gain the -f option or change to fasthalt/fastboot. The current halt and reboot programs do not run /etc/rc.shutdown, which may cause daemons not to be shut down correctly. At this time, there is no way to fix this properly, because /etc/rc.shutdown should not be run from single-user mode (if this is done, /etc/rc.d/mixer overwrites /var/db/mixer*-state with the defaults, for example) and reboot has no way to know this. When rebooting by sending a signal to init (directly, via ctrl+alt+del, by re-executing /sbin/init or via shutdown), /etc/rc.shutdown is always executed. The attached patch changes this, executing it only if /etc/rc has completed. The bigger-picture goal is to make all the common ways of shutting down (ctrl+alt+del, shutdown(8), halt(8), reboot(8), poweroff(8)) work properly both in single-user and multi-user. Currently, only ctrl+alt+del, shutdown(8) and poweroff(8) work properly in multi-user, while only halt(8) and reboot(8) work properly in single-user. The patch also fixes segfaults and other erratic behaviour if init receives SIGHUP or SIGTSTP while in single-user mode. The patch does not attempt to fix any badness with signal handlers (assumption that pointers can be read and written atomically, EINTR race condition). I believe the patch does not make this badness any worse. If the attachment does not work, the patch is also here for some time: http://www.stack.nl/~jilles/unix/init-fixes-20110102.patch -- Jilles Tjoelker --CE+1k2dSO48ffgeK Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="init-fixes-20110102.patch" Index: sbin/init/init.c =================================================================== --- sbin/init/init.c (revision 216859) +++ sbin/init/init.c (working copy) @@ -122,6 +122,7 @@ static state_func_t clean_ttys(void); static state_func_t catatonia(void); static state_func_t death(void); +static state_func_t death_single(void); static state_func_t run_script(const char *); @@ -136,6 +137,7 @@ static void transition(state_t); static state_t requested_transition; +static state_t current_state = death_single; static void setctty(const char *); static const char *get_shell(void); @@ -559,8 +561,9 @@ transition(state_t s) { + current_state = s; for (;;) - s = (state_t) (*s)(); + current_state = (state_t) (*current_state)(); } /* @@ -796,7 +799,7 @@ * Returns 0 on success, otherwise the next transition to enter: * - single_user if fork/execv/waitpid failed, or if the script * terminated with a signal or exit code != 0. - * - death if a SIGTERM was delivered to init(8). + * - death_single if a SIGTERM was delivered to init(8). */ static state_func_t run_script(const char *script) @@ -852,8 +855,8 @@ if ((wpid = waitpid(-1, &status, WUNTRACED)) != -1) collect_child(wpid); if (wpid == -1) { - if (requested_transition == death) - return (state_func_t) death; + if (requested_transition == death_single) + return (state_func_t) death_single; if (errno == EINTR) continue; warning("wait for %s on %s failed: %m; going to " @@ -1306,7 +1309,9 @@ switch (sig) { case SIGHUP: - requested_transition = clean_ttys; + if (current_state == read_ttys || current_state == multi_user || + current_state == clean_ttys || current_state == catatonia) + requested_transition = clean_ttys; break; case SIGUSR2: howto = RB_POWEROFF; @@ -1315,10 +1320,17 @@ case SIGINT: Reboot = TRUE; case SIGTERM: - requested_transition = death; + if (current_state == read_ttys || current_state == multi_user || + current_state == clean_ttys || current_state == catatonia) + requested_transition = death; + else + requested_transition = death_single; break; case SIGTSTP: - requested_transition = catatonia; + if (current_state == runcom || current_state == read_ttys || + current_state == clean_ttys || + current_state == multi_user || current_state == catatonia) + requested_transition = catatonia; break; default: requested_transition = 0; @@ -1494,9 +1506,6 @@ { struct utmpx utx; session_t *sp; - int i; - pid_t pid; - static const int death_sigs[2] = { SIGTERM, SIGKILL }; /* NB: should send a message to the session logger to avoid blocking. */ utx.ut_type = SHUTDOWN_TIME; @@ -1518,6 +1527,22 @@ /* Try to run the rc.shutdown script within a period of time */ runshutdown(); + return (state_func_t) death_single; +} + +/* + * Do what is necessary to reinitialize single user mode or reboot + * from an incomplete state. + */ +static state_func_t +death_single(void) +{ + int i; + pid_t pid; + static const int death_sigs[2] = { SIGTERM, SIGKILL }; + + revoke(_PATH_CONSOLE); + for (i = 0; i < 2; ++i) { if (kill(-1, death_sigs[i]) == -1 && errno == ESRCH) return (state_func_t) single_user; --CE+1k2dSO48ffgeK--