On Wednesday 21 September 2011 21:27:11 Jeremy Huddleston wrote: > I like the idea in theory, but I have a few requests: > > 1) Check for errors on the write. (eg: EAGAIN is not handled properly) > 2) grammer: > > +wait for a line to written out on the handlers stdout. > > > should be: > > > +wait for a line to be written out on the handlers stdout.
fixed in next patch - to follow. > > > 3) Why wait for the child to write back? I'd rather require the child exit > after handling the report and just wait(2) for that. I did not want to force the child to exit before Xorg server. But I do not have strong feeling about this. > > 4) Whether you read(2) or wait(2) from #3 above, we still have an issue where > we block the shutdown path on the actions of another process. I'd feel much > better if there were a timeout. Unfortunately, I don't see a POSIX way to do > a wait(2) with a timeout from within a signal handler. (anyone else have > thoughts?) ... I'd be fine polling it with waitpid(2) with WNOHANG and using > nanosleep(2) to give a reasonable (5 second? / configurable?) timeout. There are a number of points to we considered in coming to this design: 1. Must pre-fork to avoid needing any resources to be allocated at the point of failure 2. Do not assume anything about what the handler will get up to 3. Make the code to notify the handler depend on as little as possible 4. Make optional 5. No hardwired paths I assume that once the Xorg server has crashed its the handler's responsibility to determine the servers fate. In the cases we see when the handler is invoked by Xorg, Xorg cannot be trusted to exit. Why would I trust Xorg to timeout the handler? We for example kill -9 the server after running GDB on it to extract detailed information and reboot the system to recover. Others may choose to tell Xorg server to continue, for example to get the additional logging that sometime follows a call to xorg_backtrace. Barry
_______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
