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 792683 - ephy-profile-migrator asserts on transition from 3.24.x to 3.26.x
ephy-profile-migrator asserts on transition from 3.24.x to 3.26.x
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Profile
3.26.x
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-01-19 14:25 UTC by Andres Gomez
Modified: 2018-02-13 09:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
BT from ephy-profile-migrator (72.59 KB, text/plain)
2018-01-22 10:02 UTC, Andres Gomez
  Details
profile-migrator: Handle double bookmark timestamp migration (1.83 KB, patch)
2018-01-22 18:49 UTC, Michael Catanzaro
committed Details | Review
profile-migrator: Fix format string check added in previous commit (945 bytes, patch)
2018-01-22 19:30 UTC, Michael Catanzaro
committed Details | Review
New BT from ephy-profile-migrator (64.58 KB, text/plain)
2018-02-11 22:54 UTC, Andres Gomez
  Details

Description Andres Gomez 2018-01-19 14:25:57 UTC
I'm building epiphany (and WebKit) with JHBuild (--buildtype=debugoptimized)

With 3.26.5.1, on launching, I get:

(ephy-profile-migrator:13527): GLib-CRITICAL **: the GVariant format string '(x&s&sdbas)' has a type of '(xssdbas)' but the given value has a type of '(xssxbas)'

(ephy-profile-migrator:13527): GLib-CRITICAL **: g_variant_get: assertion 'valid_format_string (format_string, TRUE, value)' failed
Failed to run the migrator process, Web will now abort.


Also, with the same version but from Debian Testing (release build (?)), I get:

(ephy-profile-migrator:15913): GLib-CRITICAL **: the GVariant format string '(x&s&sdbas)' has a type of '(xssdbas)' but the given value has a type of '(xssxbas)'

(ephy-profile-migrator:15913): GLib-CRITICAL **: g_variant_get: assertion 'valid_format_string (format_string, TRUE, value)' failed

(ephy-profile-migrator:15913): GLib-CRITICAL **: g_variant_new_string: assertion 'g_utf8_validate (string, -1, NULL)' failed

(ephy-profile-migrator:15913): GLib-CRITICAL **: g_variant_iter_next_value: assertion 'is_valid_iter (iter)' failed

(ephy-profile-migrator:15913): GLib-CRITICAL **: g_variant_iter_free: assertion 'is_valid_heap_iter (iter)' failed
Comment 1 Andres Gomez 2018-01-19 14:26:55 UTC
I don't know if the criticals were already there by 3.24.x but, with 3.26.x is the first time it aborts at startup.
Comment 2 Michael Catanzaro 2018-01-19 18:41:20 UTC
Can you please post a backtrace taken with G_DEBUG=fatal-criticals?

We have been adding too many migrators recently... it would not be the first time that a change accidentally breaks an old migrator....
Comment 3 Bastien Nocera 2018-01-20 11:37:59 UTC
That code lives in convert_bookmark_timestamp()
Reading that function, it looks like the variant was already migrated. Could be an aborted migration.

Using g_variant_check_format_string() before g_variant_get() should be enough to avoid the error during migration.
Comment 4 Michael Catanzaro 2018-01-20 16:45:06 UTC
Thanks Bastien!

(In reply to Bastien Nocera from comment #3)
> Using g_variant_check_format_string() before g_variant_get() should be
> enough to avoid the error during migration.

Interesting, I'd never have considered this....
Comment 5 Andres Gomez 2018-01-22 10:02:39 UTC
Created attachment 367203 [details]
BT from ephy-profile-migrator
Comment 6 Andres Gomez 2018-01-22 10:03:30 UTC
(In reply to Michael Catanzaro from comment #2)
> Can you please post a backtrace taken with G_DEBUG=fatal-criticals?
> 
> We have been adding too many migrators recently... it would not be the first
> time that a change accidentally breaks an old migrator....

This is a full BT from the abort, not with G_DEBUG=fatal-criticals

"Unfortunately", I cannot reproduce any more. I don't know how it got solved ...
Comment 7 Michael Catanzaro 2018-01-22 18:49:46 UTC
The following fix has been pushed:
3e26616 profile-migrator: Handle double bookmark timestamp migration
Comment 8 Michael Catanzaro 2018-01-22 18:49:49 UTC
Created attachment 367236 [details] [review]
profile-migrator: Handle double bookmark timestamp migration

It's common for developers to run the profile migrator several times in
a row. Migrators should be robust to this. We need to ensure the
GVariant format is really the old format before using it in the
timestamp migration.
Comment 9 Michael Catanzaro 2018-01-22 19:30:15 UTC
The following fix has been pushed:
5a8870f profile-migrator: Fix format string check added in previous commit
Comment 10 Michael Catanzaro 2018-01-22 19:30:18 UTC
Created attachment 367242 [details] [review]
profile-migrator: Fix format string check added in previous commit
Comment 11 Andres Gomez 2018-01-23 21:45:15 UTC
(In reply to Andres Gomez from comment #6)
...
> "Unfortunately", I cannot reproduce any more. I don't know how it got solved
> ...

But not without price, it seems I lost my bookmarks in the process :(
Comment 12 Michael Catanzaro 2018-01-24 01:23:00 UTC
(In reply to Andres Gomez from comment #11)
> But not without price, it seems I lost my bookmarks in the process :(

That's probably a direct consequence of this bug... a table of zero bookmarks would have been written to the bookmarks file. Sorry. :/ My patch ensures the bookmarks file is no longer written to at all in this case.
Comment 13 Andres Gomez 2018-02-11 22:54:09 UTC
Created attachment 368238 [details]
New BT from ephy-profile-migrator

This doesn't seem to be completely solved.

I've now opened an instance of a web app that I had not used in some time. Ephy is 3.26.5.1 with the previous patch applied. It SIGSEVed too.
Comment 14 Michael Catanzaro 2018-02-12 17:52:53 UTC
Can you rebuild with -g for a better backtrace? The one you have there is not actionable.
Comment 15 Michael Catanzaro 2018-02-12 17:54:25 UTC
(In reply to Andres Gomez from comment #13)
> This doesn't seem to be completely solved.

Bastien, I was going to do 3.26.6 today (really!), but I think I will delay one more day because of this.
Comment 16 Andres Gomez 2018-02-13 09:36:55 UTC
(In reply to Michael Catanzaro from comment #14)
> Can you rebuild with -g for a better backtrace? The one you have there is
> not actionable.

Ouch! Checking my config this is not coming from where I claimed it was.

Closing again.

Thanks!