On Wed, Feb 24, 2010 at 03:39:20PM +0100, Paolo Bonzini wrote:
On 02/24/2010 02:06 PM, Richard W.M. Jones wrote:
The correct solution is to mask out SIGWINCH for the duration of the poll(2) system call. The per-thread mask is changed and restored immediately after the call. Since we are using pthread_sigmask, this should not affect other threads, and since we restore the signal mask immediately afterwards it should not affect the current thread visibly either.
This is interesting. No signal handler will necessarily do things like longjmp-ing out of the remote driver, so every signal could give rise to a similar bug.
Did you mean "signals handlers might longjmp out ..."? I've seen a few in my time that have done that. I notice the top Google hit for "signal handler" & "longjmp" is this warning about the practice from CERT: https://www.securecoding.cert.org/confluence/display/seccode/SIG32-C.+Do+not...
In particular, SIGCHLD would be an obvious candidate for being handled the same way, since both SIGWINCH and SIGCHLD are default-ignored signals. On the other hand, while for SIGWINCH it would be mostly harmless(*), for SIGCHLD it would leave a zombie until the remote libvirtd answers. Any ideas?
(*) Unless you have more than one thread using curses, and a thread other than the one calling libvirt has blocked SIGWINCH.
Certainly SIGCHLD could be added. I was keeping this patch minimal so it just fixes the problem I observed, to reduce the chance that a change to such a critical piece of code could break anything else. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://et.redhat.com/~rjones/libguestfs/ See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html