[PATCH-V4] xserver: Optional backtrace handler
Barry Scott
barry.scott at onelan.co.uk
Thu Oct 6 05:19:35 PDT 2011
Is there anything else I need to do to make this patch acceptable
for inclusion in Xorg?
Barry
On Thursday 29 September 2011 11:21:08 Barry Scott wrote:
> It is useful to be able to run an external program to analyse
> a crashed server process. The server will run a user supplied
> program in a subprocess of the server.
>
> The subprocess is created when the server starts up so that all
> resources needed to create the subprocess are available. It is not
> always possible to start up a process from within a crashed server.
> For example if the malloc heap is corrupt fork will not work.
>
> If the server crashes a "crash" message is sent to the handler on
> its stdin. The handler can then respond in whatever way it wishes.
> If the handler wants the server to continue, return from
> xorg_backtrace, it must write a line to its stdout.
>
> When then server exits normally the handler is sent the "exit"
> message just before the server exits.
>
> Signed-off-by: Barry Scott <barry.scott at onelan.co.uk>
> Reviewed-by: Simon Farnsworth <simon.farnsworth at onelan.co.uk>
> ---
> change from V3:
> * close backtrace_handler_stdin in child process
>
> change from V2:
> * restore trailing NL in main.c
> * beef up comments to make it clea how loops are exited
> when the child process dies
> * Use _X_HIDDEN for new symbols that are not to be exported
>
> change from V1:
>
> * check for EAGAIN and EINTR on calls to read(), write() and waitpid()
> * put -backtrace into lexical position in the command parser and usage
> * fix coding style white space and condition ordering
> * update man page with more details
> * call xorg_cleanup_backtrace() from FatalError()
>
> dix/main.c | 5 ++
> include/os.h | 3 +
> man/Xserver.man | 20 +++++++++
> os/backtrace.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> os/log.c | 3 +
> os/utils.c | 8 ++++
> 6 files changed, 159 insertions(+), 3 deletions(-)
>
> diff --git a/dix/main.c b/dix/main.c
> index 16575ce..3326e78 100644
> --- a/dix/main.c
> +++ b/dix/main.c
> @@ -147,6 +147,8 @@ int main(int argc, char *argv[], char *envp[])
>
> ProcessCommandLine(argc, argv);
>
> + xorg_init_backtrace();
> +
> alwaysCheckForInput[0] = 0;
> alwaysCheckForInput[1] = 1;
> while(1)
> @@ -354,6 +356,9 @@ int main(int argc, char *argv[], char *envp[])
> free(ConnectionInfo);
> ConnectionInfo = NULL;
> }
> +
> + xorg_cleanup_backtrace();
> +
> return 0;
> }
>
> diff --git a/include/os.h b/include/os.h
> index a553f57..00336e5 100644
> --- a/include/os.h
> +++ b/include/os.h
> @@ -541,6 +541,9 @@ extern _X_EXPORT void ErrorF(const char *f, ...) _X_ATTRIBUTE_PRINTF(1,2);
> extern _X_EXPORT void Error(const char *str);
> extern _X_EXPORT void LogPrintMarkers(void);
>
> +extern _X_HIDDEN void xorg_init_backtrace(void);
> extern _X_EXPORT void xorg_backtrace(void);
> +extern _X_HIDDEN void xorg_cleanup_backtrace(void);
> +extern _X_HIDDEN const char *BacktraceHandlerPath;
>
> #endif /* OS_H */
> diff --git a/man/Xserver.man b/man/Xserver.man
> index f743912..9def022 100644
> --- a/man/Xserver.man
> +++ b/man/Xserver.man
> @@ -100,6 +100,9 @@ specifies a file which contains a collection of authorization records used
> to authenticate access. See also the \fIxdm\fP(1) and
> \fIXsecurity\fP(__miscmansuffix__) manual pages.
> .TP 8
> +.B \-backtrace \fIbacktrace-handler-path\fP
> +specifies the path to the optional backtrace handler program. See \fBBACKTRACE HANDLER\fP below.
> +.TP 8
> .B \-br
> sets the default root window to solid black instead of the standard root weave
> pattern. This is the default unless -retro or -wr is specified.
> @@ -530,6 +533,23 @@ the following font path:
> /usr/share/fonts/default/ghostscript
> .fi
>
> +.SH BACKTRACE HANDLER
> +
> +The optional backtrace handler program will be run in a subprocess when
> +the server starts. The handler is invoked with argv[1] set to the PID
> +of the server.
> +.PP
> +The handler is expected to read a line from stdin that will tell the
> +handler what has happened to the server.
> +.PP
> +When the server is about to exit normally "exit\\n" is sent to the handler.
> +.PP
> +If the server crashes "crash\n" is sent to the handler and the server will
> +wait for a line to be written out on the handler's stdout. This causes
> +the server to return from the xorg_backtrace function. Typically after calling
> +xorg_backtrace additional error messages are logged and the server shuts down.
> +If you do not want the server to continue then kill it.
> +
> .SH FILES
> .TP 30
> .I /etc/X\fBn\fP.hosts
> diff --git a/os/backtrace.c b/os/backtrace.c
> index 7ca6dab..bcdcbf6 100644
> --- a/os/backtrace.c
> +++ b/os/backtrace.c
> @@ -28,6 +28,11 @@
> #include "os.h"
> #include "misc.h"
>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +
> #ifdef HAVE_BACKTRACE
> #ifndef _GNU_SOURCE
> #define _GNU_SOURCE
> @@ -35,7 +40,7 @@
> #include <dlfcn.h>
> #include <execinfo.h>
>
> -void xorg_backtrace(void)
> +void xorg_builtin_backtrace(void)
> {
> void *array[64];
> const char *mod;
> @@ -180,7 +185,7 @@ static int xorg_backtrace_pstack(void) {
>
> # if defined(HAVE_PSTACK) || defined(HAVE_WALKCONTEXT)
>
> -void xorg_backtrace(void) {
> +void xorg_builtin_backtrace(void) {
>
> ErrorF("\nBacktrace:\n");
>
> @@ -207,7 +212,119 @@ void xorg_backtrace(void) {
> # else
>
> /* Default fallback if we can't find any way to get a backtrace */
> -void xorg_backtrace(void) { return; }
> +void xorg_builtin_backtrace(void) { return; }
>
> # endif
> #endif
> +
> +const char *BacktraceHandlerPath = NULL;
> +
> +static int backtrace_handler_stdin;
> +static int backtrace_handler_stdout;
> +static pid_t child_pid;
> +
> +static const char cmd_crash[] = "crash\n";
> +static const char cmd_exit[] = "exit\n";
> +
> +static void report_init_error(const char *msg) {
> + /* prevent any further use of the handler */
> + BacktraceHandlerPath = NULL;
> + FatalError("%s - errno %d", msg, errno);
> +}
> +
> +void xorg_init_backtrace(void) {
> + char pid_buffer[32];
> + int pipe_fds[2];
> + int child_stdin;
> + int child_stdout;
> +
> + if (!BacktraceHandlerPath)
> + return;
> +
> + if (pipe(pipe_fds) != 0)
> + report_init_error("failed to create pipe");
> + backtrace_handler_stdin = pipe_fds[1];
> + child_stdin = pipe_fds[0];
> +
> + if (pipe(pipe_fds) != 0)
> + report_init_error("failed to create pipe");
> + backtrace_handler_stdout = pipe_fds[0];
> + child_stdout = pipe_fds[1];
> +
> + /* server pid will be passed in argv[1] */
> + snprintf( pid_buffer, sizeof(pid_buffer), "%d", getpid());
> +
> + child_pid = fork();
> + if (child_pid < 0) {
> + report_init_error("failed to fork");
> + } else {
> + if (child_pid == 0) {
> + /* setup stdin, stdout to point to the pipes */
> + dup2(child_stdin, STDIN_FILENO);
> + dup2(child_stdout, STDOUT_FILENO);
> +
> + /* close all other fds to the pipes */
> + close(child_stdin);
> + close(child_stdout);
> + close(backtrace_handler_stdin);
> + close(backtrace_handler_stdout);
> +
> + execl(BacktraceHandlerPath, BacktraceHandlerPath, pid_buffer, NULL);
> + exit(1);
> + } else {
> + if (close(child_stdin) != 0)
> + report_init_error("failed to close pipe fd");
> + if (close(child_stdout) != 0)
> + report_init_error("failed to close pipe fd");
> + }
> + }
> +}
> +
> +void xorg_backtrace(void) {
> + if (BacktraceHandlerPath) {
> + int status;
> +
> + /* check that the handler process is still running */
> + if (waitpid(child_pid, &status, WNOHANG) == 0) {
> + char buffer[1];
> +
> + /* tell backtrace handler xorg has crashed */
> + while (write(backtrace_handler_stdin, cmd_crash, sizeof(cmd_crash ) - 1) < 0)
> + if (errno != EAGAIN && errno != EINTR)
> + break;
> +
> + /* exit to caller after backtrace handler writes anything to stdout */
> + while (read(backtrace_handler_stdout, buffer, sizeof(buffer)) < 0)
> + if (errno != EAGAIN && errno != EINTR)
> + break;
> +
> + } else {
> + /* handler process has quit? run builtin handler */
> + xorg_builtin_backtrace();
> + }
> + } else {
> + xorg_builtin_backtrace();
> + }
> +}
> +
> +void xorg_cleanup_backtrace(void) {
> + if (BacktraceHandlerPath) {
> + int status;
> +
> + BacktraceHandlerPath = NULL;
> +
> + /* tell backtrace handler xorg is about to exit cleanly
> + EPIPE is returned if the child process closes the pipe
> + (which happens when the process exits) and the loop will exit */
> + while (write(backtrace_handler_stdin, cmd_exit, sizeof(cmd_exit ) - 1) < 0)
> + if (errno != EAGAIN && errno != EINTR)
> + break;
> +
> + /* If the child has already exited waitpid return with ECHILD
> + and the loop will exit */
> + while (waitpid(child_pid, &status, 0) < 0)
> + if (errno != EINTR)
> + break;
> +
> + }
> +}
> diff --git a/os/log.c b/os/log.c
> index f519762..7b3409a 100644
> --- a/os/log.c
> +++ b/os/log.c
> @@ -523,6 +523,9 @@ FatalError(const char *f, ...)
> va_list args;
> static Bool beenhere = FALSE;
>
> + /* make sure any backtrace handler process is told to exit */
> + xorg_cleanup_backtrace();
> +
> if (beenhere)
> ErrorF("\nFatalError re-entered, aborting\n");
> else
> diff --git a/os/utils.c b/os/utils.c
> index 36cb46f..78c39e6 100644
> --- a/os/utils.c
> +++ b/os/utils.c
> @@ -470,6 +470,7 @@ void UseMsg(void)
> ErrorF("-ac disable access control restrictions\n");
> ErrorF("-audit int set audit trail level\n");
> ErrorF("-auth file select authorization file\n");
> + ErrorF("-backtrace string path of program to run to handle a server crash\n");
> ErrorF("-br create root window with black background\n");
> ErrorF("+bs enable any backing store support\n");
> ErrorF("-bs disable any backing store support\n");
> @@ -616,6 +617,13 @@ ProcessCommandLine(int argc, char *argv[])
> else
> UseMsg();
> }
> + else if ( strcmp( argv[i], "-backtrace") == 0)
> + {
> + if(++i < argc)
> + BacktraceHandlerPath = argv[i];
> + else
> + UseMsg();
> + }
> else if ( strcmp( argv[i], "-br") == 0) ; /* default */
> else if ( strcmp( argv[i], "+bs") == 0)
> enableBackingStore = TRUE;
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20111006/e7771d06/attachment.html>
More information about the xorg-devel
mailing list