GNU bug report logs - #24041
25.1.50; xwidget + -nw mode gives segfault

Previous Next

Full log

View this message in rfc822 format

From: Robert Cochran <robert-emacs <at> cochranmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 24041 <at> debbugs.gnu.org, shanemhansen <at> gmail.com, Robert Cochran <robert-emacs <at> cochranmail.com>
Subject: bug#24041: 25.1.50; xwidget + -nw mode gives segfault
Date: Mon, 22 Aug 2016 11:30:15 -0700

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Robert Cochran <robert-emacs <at> cochranmail.com>
>> Cc: Robert Cochran <robert-emacs <at> cochranmail.com>,  shanemhansen <at> gmail.com,  24041 <at> debbugs.gnu.org
>> Date: Sun, 21 Aug 2016 19:12:41 -0700
>> 
>> Anyways, I have a patch, below, that's my first stab at solving the
>> problem. All it does is call `check_x_display_info` with the current
>> frame and allows any resulting error signals to propagate back up.
>> 
>> Probably not the most elegant solution, but I'm not entirely clear what
>> can and can't be done from within the Emacs C core. Suggestions are very
>> welcome.
>
> My only comment is that you could call check_x_display_info with Qnil
> as its argument.

I did think about that. But then it arguably does the wrong thing:
`check_x_display_info` with `Qnil` only signals an error when there have
never been X windows, eg, opening and closing an X window satisfies the
check from then on. It no longer crashes in that instance, but I
personally don't think that's the right behavior; if my starting frame
isn't capable of displaying an xwidget, say so! Hence checking with the
current frame.

That, of course, is just my opinion.

> Otherwise, LGTM, thanks.  And I see nothing inelegant in this patch.

Thanks for your reassurance! My one gripe about this patch is that I
didn't figure out how to kill the buffer after xwidget creation failure
(leaving it seems rather ugly IMO), but I just now realized what I can
do. As long as it's not considered wrong to kill a mode's buffer on
error, would you also consider this patch to go along with it?

-----

[0001-Kill-the-webkit-browser-buffer-if-an-error-occurs.patch (text/x-patch, inline)]

From e58ed09121969febde5bfc2206c14f4a7806c323 Mon Sep 17 00:00:00 2001
From: Robert Cochran <robert-git <at> cochranmail.com>
Date: Mon, 22 Aug 2016 11:21:01 -0700
Subject: [PATCH] Kill the webkit browser buffer if an error occurs

* lisp/xwidget.el (xwidget-webkit-new-session): kill webkit buffer on
  error
---
 lisp/xwidget.el | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lisp/xwidget.el b/lisp/xwidget.el
index 7a0ca8b..e925c9a 100644
--- a/lisp/xwidget.el
+++ b/lisp/xwidget.el
@@ -427,11 +427,16 @@ xwidget-webkit-new-session
        xw)
     (setq xwidget-webkit-last-session-buffer (switch-to-buffer
                                               (get-buffer-create bufname)))
-    (insert " 'a' adjusts the xwidget size.")
-    (setq xw (xwidget-insert 1 'webkit  bufname 1000 1000))
-    (xwidget-put xw 'callback 'xwidget-webkit-callback)
-    (xwidget-webkit-mode)
-    (xwidget-webkit-goto-uri (xwidget-webkit-last-session) url)))
+    (condition-case err
+        (progn
+          (insert " 'a' adjusts the xwidget size.")
+          (setq xw (xwidget-insert 1 'webkit  bufname 1000 1000))
+          (xwidget-put xw 'callback 'xwidget-webkit-callback)
+          (xwidget-webkit-mode)
+          (xwidget-webkit-goto-uri (xwidget-webkit-last-session) url))
+      ;; On error, remove webkit buffer and resignal
+      (error (kill-buffer bufname)
+             (signal (car err) (cdr err))))))
 
 
 (defun xwidget-webkit-goto-url (url)
-- 
2.7.4

[Message part 3 (text/plain, inline)]

-----

Otherwise, with or without the new patch, I have no issues if you have
no issues.

Thanks,
-- 
~Robert Cochran

GPG Fingerprint - E778 2DD4 FEA6 6A68 6F26  AD2D E5C3 EB36 4886 8871

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.