<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN" "http://www.w3.org/TR/REC-html40/strict.dtd"> <html><head><meta name="qrichtext" content="1" /><style type="text/css"> p, li { white-space: pre-wrap; } </style></head><body style=" font-family:'Monospace'; font-size:10pt; font-weight:400; font-style:normal;"> On Wednesday 28 September 2011 18:37:04 Jeremy Huddleston wrote: > After some thought, I think it's a good idea to not wait for the handler to exit. It may want to do some extra processing that it doesn't need the process hanging around for. > > You probably meant one of these to be backtrace_handler_stdin: > > + close(backtrace_handler_stdout); > > + close(backtrace_handler_stdout); Opss... V4 will fix this > > I'd like to see a followup to set BacktraceHandlerPath from xorg.conf Can I leave that to someone else? Barry > > On Sep 28, 2011, at 3:09 AM, 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 the server exits normally the handler is sent the "exit" > > message just before the server exits. > > > > Signed-off-by: Barry Scott <barry.scott@onelan.co.uk> > > Reviewed-by: Simon Farnsworth <simon.farnsworth@onelan.co.uk> > > --- > > 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..f9114eb 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_stdout); > > + 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; > > > </body></html>