issue514: Segmentation fault in Firefox >= 53

Priority: bug Status: chatting

Issues
Show All
Search

Login




Register
Lost your login?
Files
File name Uploaded Type Edit Remove
0001-remove-deprecated-use-of-for-each-.-in.patch parkouss, 2017-05-06.07:45:51 text/x-patch
conk.bt MarkEichin, 2017-05-01.21:35:45 application/octet-stream
Messages
msg1336 (view) Author: scottjad Date: 2017-04-18.13:37:56 
Starting in Firefox 53, Conkeror will crash when links like this are clicked:

<a href="#" target="foo">crashes</a>

This link does not crash:

<a href="#" target="foo" rel="noopener" >doesn't crash<a>

And if the second link is clicked before the first link, the first link will not crash.

Here is a backtrace from a crash:

  #0  0x00007fffe9ed179f in nsGlobalWindow::SetOpenerWindow(nsPIDOMWindowOuter*, bool) [clone .cold.401]
() from /home/scott/src/firefox/dev/libxul.so
#1  0x00007fffec2010ef in nsWindowWatcher::ReadyOpenedDocShellItem(nsIDocShellTreeItem*,
nsPIDOMWindowOuter*, bool, bool, mozIDOMWindowProxy**) ()
   from /home/scott/src/firefox/dev/libxul.so
#2  0x00007fffec1ff0c0 in nsWindowWatcher::OpenWindowInternal(mozIDOMWindowProxy*, char const*, char
const*, char const*, bool, bool, bool, nsIArray*, bool, bool, nsIDocShellLoadInfo*, mozIDOMWindowProxy**) ()
   from /home/scott/src/firefox/dev/libxul.so
#3  0x00007fffeb2adf3c in nsWindowWatcher::OpenWindow2(mozIDOMWindowProxy*, char const*, char const*,
char const*, bool, bool, bool, nsISupports*, bool, bool, nsIDocShellLoadInfo*, mozIDOMWindowProxy**)
[clone .cold.96] ()
   from /home/scott/src/firefox/dev/libxul.so
#4  0x00007fffe9ed4867 in nsGlobalWindow::OpenInternal(nsAString_internal const&, nsAString_internal
const&, nsAString_internal const&, bool, bool, bool, bool, bool, nsIArray*, nsISupports*,
nsIDocShellLoadInfo*, bool, nsPIDOMWindowOuter**) [clone .cold.510] () from
/home/scott/src/firefox/dev/libxul.so
#5  0x00007fffe9ee07bc in nsGlobalWindow::OpenNoNavigate(nsAString_internal const&, nsAString_internal
const&, nsAString_internal const&, nsPIDOMWindowOuter**) ()
msg1337 (view) Author: scottjad Date: 2017-05-01.21:08:17 
Maybe a related Firefox bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1334086
msg1338 (view) Author: MarkEichin Date: 2017-05-01.21:35:45 
Attached backtrace from ubuntu 16.04 (firefox 53.0+build6-0ubuntu0.16.04.1, conkeror
1.0.3+git170412+2207-~nightly1) triggered by "click on any link in a mail message in Gmail", for another
example.  (Rolling back to firefox-esr worked, confirming at least that the problem is along that upgrade
boundary.)
msg1339 (view) Author: parkouss Date: 2017-05-06.07:45:51 
I have the exact same issue. I built the nightly (from mozilla-central) firefox, and the issue is still
there - here is the exact revision I built:

changeset:   356738:23fe0b76a018
tag:         tip
fxtree:      central
user:        ffxbld
date:        Fri May 05 08:05:06 2017 -0700

As said in the first comment, the issue can be seen with a simple html page:

<body>
  Starting in Firefox 53, Conkeror will crash when links like this are clicked:
  <a href="#" target="foo">crashes</a>
  This link does not crash:
  <a href="#" target="foo" rel="noopener" >doesn't crash<a>
</body>


Using the mozregression tool, I narrowed down to the following changesets:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=944cb0fd05526894fcd90fbe7d1e625ee53cd73d&tochange=1b170b39ed6bdbde366233ab84594bdaaa960a5a

on mozilla-central, which leads me to:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3f15ff10cb267d4289655b2e3cf0a0dee48e7c0b&tochange=2a31079dae258444321138690fa68de6b3ddf06a

on mozilla-inbound.

Note that I had to replace for each ... in constructions with for ... of since for each does not work
anymore on firefox nightlies. I attach the patch I wrote - BTW, maybe the patch should be landed in
conkeror master, according to
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of for ... of is
working since firefox 13, except for a bug with const keyword which has been resolved in firefox 51.
Anyway I don't see any use of const with the loops I fixed, so it should be fine I think?

I will open a bug on the mozilla side.
msg1340 (view) Author: parkouss Date: 2017-05-06.08:21:15 
firefox issue filled: https://bugzilla.mozilla.org/show_bug.cgi?id=1362661
msg1342 (view) Author: parkouss Date: 2017-05-13.05:42:27 
From what I can understand in the referenced bug, somewhere we need to call
browser.presetOpenerWindow(aOpener); in the openuri function.

I tried to put the following patch:
modified   modules/content-buffer.js
@@ -615,6 +615,9 @@ browser_dom_window.prototype = {
 
         /* Determine the opener buffer */
         var opener = get_buffer_from_frame(this.window, aOpener);
+        if (aOpener) {
+            this.browser.presetOpenerWindow(aOpener);
+        }
 
         switch (browser_default_open_target) {
         case OPEN_CURRENT_BUFFER:

And that fixes the crash - though I have issues to open links from gmail for
example, a dialog box is shown telling that there is probably a popup blocker
preventing to open the page and the page is not displayed;


I am not sure if I can help more with my current knowledge, as I don't really
understand what I am doing here. :) Scott, maybe you understand better than me
and you have some time to look into it ? Else I might ask some help on the
mozilla irc.
msg1343 (view) Author: scottjad Date: 2017-05-14.22:56:25 
Julien,

Well if that prevents conkeror from crashing that's pretty nice even if it doesn't work in the situations
where it would have crashed.

Unfortunately I don't understand the issue any better than you. I did try a few minor changes but wasn't
able to get a fix.

I'haven't had any luck with the mozilla IRC in the past, and I don't think there are going to be many
people willing to take a close enough look at a project they don't use.

Our best hope is that a conkeror user or past contributor out there decides to do a deep dive into this.
As more and more people upgrade to ff53, or when ff59 comes out and ESR users upgrade, there may be
someone who figures out a fix and submits it.

You could send an email to the conkeror mailing list letting some of these people know about the issue.

If no fix is forthcoming, you may consider running firefox 52 which luckily is an ESR so should be
supported until ff59.

Cheers
msg1344 (view) Author: retroj Date: 2017-05-15.12:59:52 
Julien, your patch above is just inserting an error into browser_dom_window.openURI - there is no
property 'this.browser'.  So any error, or perhaps a simple 'return null;' at that point may reproduce
the 'fix' you see.  I would be curious to know what behavior results from 'return null;' at that point.

From my reading though, what Michael Layzell indicated is that opener needs to be set on the new window,
not the old one.  Scott and I tried a couple of ways of doing exactly that, and the crash remained, so in
my view, this crash that we're seeing is the one from
https://bugzilla.mozilla.org/show_bug.cgi?id=1334086 not the one from
https://bugzilla.mozilla.org/show_bug.cgi?id=1316104
msg1345 (view) Author: scottjad Date: 2017-06-17.01:57:26 
I think this issue might be fixed in FF 54, since my minimal test case no longer produces the crash.
History
Date User Action Args
2017-06-17 01:57:26scottjadsetmessages: + msg1345
2017-05-15 12:59:52retrojsetmessages: + msg1344
2017-05-14 22:56:25scottjadsetmessages: + msg1343
2017-05-13 05:42:28parkousssetmessages: + msg1342
2017-05-06 08:21:15parkousssetmessages: + msg1340
2017-05-06 07:45:51parkousssetfiles: + 0001-remove-deprecated-use-of-for-each-.-in.patch
messages: + msg1339
2017-05-01 21:35:45MarkEichinsetfiles: + conk.bt
messages: + msg1338
2017-05-01 21:08:17scottjadsetstatus: unread -> chatting
messages: + msg1337
2017-04-18 13:37:56scottjadcreate