emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Interferences between xwidgets and async processes?


From: Akira Kyle
Subject: Re: [PATCH] Interferences between xwidgets and async processes?
Date: Thu, 12 Nov 2020 12:25:21 -0700
User-agent: mu4e 1.4.13; emacs 28.0.50

On Mon, Nov 09, 2020 at 06:52 AM, Lars Ingebrigtsen 
<larsi@gnus.org> wrote:
* src/xwidget.c (make-xwidget): Save and restore Emacs SIGCHLD signal
handler since glib doesn't (but should) do this.
Thanks for the analysis and the patch for this problem.  I've 
now
applied it to Emacs 28, and it fixes the issue here.
It seems I jumped the gun on this one. While it does fix the 
reported issue, it leaves WebKit process zombies. The issue is 
deeper than I previously thought and it seems that even 
disregarding the xwidget feature, the current interplay between 
Emacs' and glib's signal handling is fundamentally broken since 
glib commit 2e471acf. The relevant glib issue that resulted in 
that commit is here [1].
Essentially Emacs tries to capture glib's signal handler, 
g_unix_signal_handler, into lib_child_handler in process.c so that 
when Emacs installs its own signal handler, deliver_child_signal, 
overwriting glib's, after deliver_child_signal handles waiting on 
any process Emacs starts, it calls  g_unix_signal_handler so glib 
has the chance to wait on any process it may be handling. The 
comment before defining lib_child_handler in process.c explains 
this intent. However since glib 2e471acf breaks all this since 
glib now may install its signal handler multiple times instead of 
just once and will reset the signal handler to SIG_DFL if it 
doesn't have any watchers on that signal. I'm somewhat surprised 
this hasn't caused other issues so far, but I guess Emacs doesn't 
use glib for any process handling and own its own gtk isn't 
spawning any of its own processes on behalf of Emacs.
The attached patch fixes this but only in a temporary way. Any 
Emacs code which uses gtk or glib to spawn a process will suffer 
the same bugs as the reported xwidget bug unless they are also 
fixed in a similar way to the patch I previously sent, however 
even then my previous patch has potential race conditions as an 
Emacs managed subprocess may throw SIGCHLD between glib installing 
g_unix_signal_handler in ref_unix_signal_handler_unlocked and 
Emacs calling sigaction to restore deliver_child_signal which will 
leave the process zombied so Emacs will need to block SIGCHLD 
during this, something my previous patch failed to consider.
I think that this should ultimately be fixed in glib by making it 
be a better signal handling citizen and do what Emacs does by 
saving any existing signal handler and call it from its own signal 
handler. I suppose I might be able to send them a patch if that 
seems like the best course of action here.
[1] https://gitlab.gnome.org/GNOME/glib/-/issues/733

Attachment: 0001-Work-around-glib-messing-with-signal-handlers-more-t.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]