search

akirakyle / emacs-webkit

An Emacs Dynamic Module for WebKit, aka a fully fledged browser inside emacs
GNU General Public License v3.0
419 stars 24 forks source link

The hack for SIGCHLD does not work for me #28

Open blahgeek opened 3 years ago

blahgeek commented 3 years ago

In my system, the hack for SIGCHLD does not work. The signal handler for SIGCHLD is still modified after opening webkit buffers, which makes all later sub-processes becoming zombies.

I did some debugging and found out that the function webkit_web_view_new_with_context modified the signal handler. I'm using latest gtk libraries in arch linux (webkit2gtk 2.32.0-2, gtk3 1:3.24.29-1), I'm not sure if it's related to the version.

nicolasavru commented 3 years ago

Heh, I just started using emacs-webkit (also on Arch) and was about to start debugging this myself. Moving struct sigaction old_action; sigaction (SIGCHLD, NULL, &old_action); up to right above WebKitWebContext *context = webkit_web_context_new (); seems to have solved the problem for me. Webkit seems to work fine, but I don't know anything about the internals of webkit, so I don't know what impact this will have there.

The current code seems to have from xwidget.c[1]. Here's the commit: [2]. Emacs bug report: [3] and [4]. The latter thread in particular has interesting discussion, another patch was applied to process.c in emacs to try to address this, but here's the underlying glib bug: [5].

[1] https://git.savannah.gnu.org/cgit/emacs.git/tree/src/xwidget.c#n133 [2] https://git.savannah.gnu.org/cgit/emacs.git/commit/src/xwidget.c?id=a05810b2d5f1a87ad1b20c4e61adc464a31e315d [3] https://lists.gnu.org/archive/html/emacs-devel/2020-10/msg01709.html [4] https://lists.gnu.org/archive/html/emacs-devel/2020-11/msg00165.html [5] https://gitlab.gnome.org/GNOME/glib/-/issues/2216

blahgeek commented 3 years ago

I just tested the latest master version of emacs with xwidget enabled. Xwidget-webkit also have the same issue.

blahgeek commented 3 years ago

Moving struct sigaction old_action; sigaction (SIGCHLD, NULL, &old_action); up to right above WebKitWebContext *context = webkit_web_context_new (); seems to have solved the problem for me. Webkit seems to work fine, but I don't know anything about the internals of webkit, so I don't know what impact this will have there.

Yes I can also confirm this works. I think there may be two potential problems:

  1. After we restored the sighandler, webkit may not work properly because it cannot recieve SIGCHLD signal
  2. SIGCHLD signal may be triggered during this function, before we restore the sighandler, then emacs would miss it

For 1, I believe it's fixed by this patch[1]?

For 2, maybe we can always call the sighandler at the end of the function? According to the current emacs sighandler implementation, it should be a no-op if it's not actually required.

[1] https://lists.gnu.org/archive/html/emacs-devel/2020-11/msg00508.html

akirakyle commented 3 years ago

It's been awhile since looking into all of this. If I'm recalling correctly, at one point I got discouraged by the hack Emacs uses since it was conflicting with some deep logic in the child process handling of webkit's bubblewrap. A proper fix for all of this should happen at some point, but requires diving deeply into the internals of Emacs, gtk, glib, and webkit and it's hard to reason about signal handling without introducing race conditions and other subtle bugs. Sorry my time has been extremely limited lately I'm not sure I'll be able to make any progress on this anytime soon, but as always, PRs are welcome.

akirakyle commented 2 years ago

I can confirm I am currently experiencing this bug so it will be the first thing I try to fix when I have time.