After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 768250 - Crash when restoring session
Crash when restoring session
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: General
3.20.x (obsolete)
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-06-30 14:48 UTC by Carlos Garnacho
Modified: 2016-10-17 12:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
embed: Bail out early on invalid uri (918 bytes, patch)
2016-06-30 14:49 UTC, Carlos Garnacho
none Details | Review
embed: Check for NULL SoupURI when trying to load invalid url (1.43 KB, patch)
2016-06-30 15:24 UTC, Carlos Garnacho
committed Details | Review
session: Add a safety check (1.94 KB, patch)
2016-10-17 12:22 UTC, Michael Catanzaro
committed Details | Review

Description Carlos Garnacho 2016-06-30 14:48:44 UTC
Last evening I had to close ephy because of a tab being blocked (I think down to dri3/libva being funny here, not the only issue I have around it).

Thing is, when I restarted the ephy, I was greeted each time with this backtrace:

  • #0 update_security_status_for_committed_load
    at ephy-web-view.c line 1430
  • #1 load_changed_cb
    at ephy-web-view.c line 1491
  • #2 g_closure_invoke
    at gclosure.c line 804
  • #3 signal_emit_unlocked_R
    at gsignal.c line 3629
  • #4 g_signal_emit_valist
    at gsignal.c line 3385
  • #5 g_signal_emit
    at gsignal.c line 3441
  • #6 WebKit::WebPageProxy::didCommitLoadForFrame(unsigned long, unsigned long, WTF::String const&, bool, bool, unsigned int, WebCore::CertificateInfo const&, bool, WebKit::UserData const&)
    from /lib64/libwebkit2gtk-4.0.so.37
  • #7 void IPC::handleMessage<Messages::WebPageProxy::DidCommitLoadForFrame, WebKit::WebPageProxy, void
  • #8 WebKit::WebPageProxy::didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&)
    from /lib64/libwebkit2gtk-4.0.so.37
  • #9 IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::MessageDecoder&)
    from /lib64/libwebkit2gtk-4.0.so.37
  • #10 WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection&, IPC::MessageDecoder&)
    from /lib64/libwebkit2gtk-4.0.so.37
  • #11 IPC::Connection::dispatchMessage(std::unique_ptr<IPC::MessageDecoder, std::default_delete<IPC::MessageDecoder> >)
    from /lib64/libwebkit2gtk-4.0.so.37
  • #12 IPC::Connection::dispatchOneMessage()
    from /lib64/libwebkit2gtk-4.0.so.37
  • #13 WTF::RunLoop::performWork()
    from /lib64/libjavascriptcoregtk-4.0.so.18
  • #14 WTF::RunLoop::RunLoop()::{lambda(void*)#1}::_FUN(void*)
    from /lib64/libjavascriptcoregtk-4.0.so.18
  • #15 g_main_dispatch
    at gmain.c line 3200
  • #16 g_main_context_dispatch
    at gmain.c line 3846
  • #17 g_main_context_iterate
    at gmain.c line 3919
  • #18 g_main_context_iteration
    at gmain.c line 3980
  • #19 g_application_run
    at gapplication.c line 2381
  • #20 main
    at ephy-main.c line 479

First thing worth pointing out is the invalid "http:/" uri, I can't assure that this invalid url belongs to the tab that caused the blocking or not, nor I can assure that the tab was previously like that when the session was saved. 

One thing I can assure is, I didn't write that, nor reached there through clicking. I sometimes see blank tabs after restoring a session (unclean exits might be related?), I wonder if this is a related issue.

Aside of these issues, ephy should startup :). Attaching a small patch to bail out if the SoupURI could not be constructed.

PS. in case it was not clear, I've got restore-session-policy=always in settings
PPS. I'm reporting this to 3.20, the issue might apply to master, but I've been admittedly lazy at compiling WebKit...
Comment 1 Carlos Garnacho 2016-06-30 14:49:06 UTC
Created attachment 330664 [details] [review]
embed: Bail out early on invalid uri

Fixes crash a bit later when accessing soup_uri memory. If
there is no uri, there's not much to do here...
Comment 2 Michael Catanzaro 2016-06-30 15:04:56 UTC
Review of attachment 330664 [details] [review]:

I wish we knew where the http:/ URI was coming from. Oh well, thanks for your patch.

::: embed/ephy-web-view.c
@@ +1425,3 @@
   soup_uri = soup_uri_new (uri);
 
+  if (!soup_uri)

In this case we should probably still update the security level to EPHY_SECURITY_LEVEL_LOCAL_PAGE (no security indicator), just in case this somehow happens when the security level is something else, so I would instead add the !soup_uri check to the first condition down below, and then just null-check it before freeing it.
Comment 3 Carlos Garnacho 2016-06-30 15:24:27 UTC
Created attachment 330669 [details] [review]
embed: Check for NULL SoupURI when trying to load invalid url

Fixes crashes when accessing soup_uri memory. If there is no valid
uri, just set the "local page" security level.
Comment 4 Carlos Garnacho 2016-06-30 15:32:40 UTC
(In reply to Michael Catanzaro from comment #2)
> Review of attachment 330664 [details] [review] [review]:
> 
> I wish we knew where the http:/ URI was coming from. Oh well, thanks for
> your patch.

I agree, and would like to track this down (also for the lost blank tabs), first I'd like to find out if it clean/unclean ephy shutdowns influenced. Sadly, session durability/stability is not one of my dev machine features :)
Comment 5 Michael Catanzaro 2016-06-30 16:16:47 UTC
Review of attachment 330669 [details] [review]:

OK
Comment 6 Michael Catanzaro 2016-06-30 16:16:59 UTC
Also for gnome-3-20
Comment 7 Carlos Garnacho 2016-06-30 16:24:28 UTC
Cheers! pushing to both branches

Attachment 330669 [details] pushed as 8d9adfe - embed: Check for NULL SoupURI when trying to load invalid url
Comment 8 Michael Catanzaro 2016-10-16 03:47:29 UTC
(In reply to Carlos Garnacho from comment #4)
> I agree, and would like to track this down (also for the lost blank tabs),
> first I'd like to find out if it clean/unclean ephy shutdowns influenced.
> Sadly, session durability/stability is not one of my dev machine features :)

OK, I have a reproducer. It's not a very good reproducer, though. Create a web extension that calls some function that cannot be resolved at link time. In my case, I built Epiphany against a libhttpseverywhere API that got removed. Now the web process cannot start. When you close Epiphany and reopen, http:/ gets saved for each tab in the session file. This shouldn't happen.
Comment 9 Carlos Garnacho 2016-10-16 08:30:26 UTC
I never had a good reproducer myself either :(. AFAICS it usually happens on old opened tabs that go through several close/reboot/(session) kill/OOM cycles (my tab hoarding doesn't help :).

Now, I'm pretty sure that nowadays I see more often just "Blank page" tabs that were supposedly restored pages than "http:/" ones.
Comment 10 Michael Catanzaro 2016-10-17 12:22:40 UTC
The following fix has been pushed:
e212e2f session: Add a safety check
Comment 11 Michael Catanzaro 2016-10-17 12:22:44 UTC
Created attachment 337842 [details] [review]
session: Add a safety check

Never replace a good session file with one that's known to be broken.