GNOME Bugzilla – Bug 696020
[WK2] When ran to open an URI in an existing instance, creates a WebProcess which then crashes because the UI process is gone
Last modified: 2013-08-01 13:22:20 UTC
I added the core dumps trick that's used in the (Igalia) WebKit buildbots to my machine, and I use a very simple script to notify me of crashes. I noticed a notification for the WebProcess crashing happened a lot, and I nailed it down to the condition described in the summary. The problem is we delay noticing we're just trying to open a link in an existing instance for quite a bit, and a *lot* of stuff is initialized as if we were going to run a full instance. The creation of the WebProcess is triggered by that, but we then notice the existing instance, tell it to do stuff over D-Bus and exit - the WebProcess tries to create the socket for the specified fd and crashes with a g_error. The one that triggers the WebProcess creation is EphyEmbedPrefs: Breakpoint 3, WorkQueue::dispatch(WTF::Function<void ()> const&) (this=0x5982e0, function=...) at ../src/StableWebKit/Source/WebKit2/Platform/gtk/WorkQueueGtk.cpp:202 202 GRefPtr<GSource> dispatchSource = adoptGRef(g_idle_source_new()); (gdb) bt
+ Trace 231656
I think we can do something on the WebProcess to make make this situation not critical (by making it not crash if the fd disappeared), but I think we could also improve epiphany to notice we should just send a D-Bus message earlier, too. Opinions?
This is the error that Epiphany prints, btw: (WebKitWebProcess:16101): GLib-GIO-ERROR **: creating GSocket from fd 11: Bad file descriptor
Yes, I've seen this before sometimes for wk2 unit tests that run very fast, the ui process is gone when the web process is creating the socket. So, I agree that this can be improved in webkit, but we can also try to make ephy a bit smarter, I guess.
Maybe we can just init the prefs and all other stuff related to the web process that we are setting in ephy-main right before the prefs_init in ephy_shell_startup.
Created attachment 239280 [details] [review] Patch
Works nicely, for me =)
Review of attachment 239280 [details] [review]: ::: src/ephy-shell.c @@ +261,3 @@ + webkit_web_context_set_disk_cache_directory (webkit_web_context_get_default (), + disk_cache_dir); + g_free (disk_cache_dir); I'd say we have the opportunity to make this a bit cleaner now. The cache dir and extensions dir settings should probably go inside ephy_embed_prefs_init(), right? And for bonus points you could dump all the env vars into ephy_shell_set_environment () or something like that.
(In reply to comment #6) > Review of attachment 239280 [details] [review]: > > ::: src/ephy-shell.c > @@ +261,3 @@ > + webkit_web_context_set_disk_cache_directory (webkit_web_context_get_default > (), > + disk_cache_dir); > + g_free (disk_cache_dir); > > I'd say we have the opportunity to make this a bit cleaner now. The cache dir > and extensions dir settings should probably go inside ephy_embed_prefs_init(), > right? I don't think so, they are not settings. They will be moved to where WebKitWebContext are created when we support multiple web processes. Since we only support one in this moment, I would leave it here. I could use a helper function for this too, though, something like ephy_shell_setup_default_web_context() or something similar. > And for bonus points you could dump all the env vars into > ephy_shell_set_environment () or something like that. Yes, I thought about it indeed, but in the end decided to just move the code. I'll do it.
Pushed an updated patch.
Reopening, since there's code in ephy_embed_init() that could spawn a web process.
Created attachment 240061 [details] [review] Unify all web context setup in EphyEmbedShell on primary instance startup
Review of attachment 240061 [details] [review]: OK, makes sense. Push to both branches.
Pushed to master, will push to the stable branch after fixing all the conflicts due to the #ifdefs
This might fix bug #666326.
hmm, we don't really need this in the stable branch. This is a problem in master (see bug #666326) because of the ephy embed single removal. In the stable branch, initialization happens in ephy_embed_single_intialize() that is always called after the ephy_shell_startup().
So what is exactly missing here? And what's the status of the attached patches?
This is fixed.