WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
175719
[GTK][WPE] ASSERTION FAILED: !isOpen() in WebKit::IconDatabase::~IconDatabase()
https://bugs.webkit.org/show_bug.cgi?id=175719
Summary
[GTK][WPE] ASSERTION FAILED: !isOpen() in WebKit::IconDatabase::~IconDatabase()
Carlos Garcia Campos
Reported
2017-08-18 06:09:13 PDT
This is happening always when running /webkit2/WebKitFaviconDatabase/favicon-database-test in debug builds. The last step we do is removing all icons, then the test finishes, which destroys the WebKitFaviconDatabase object that closes the icon database on dispose. The problem is that removing all icons schedules a main thread notification and IconDatabase is not considered closed until all main thread callback have been dispatched. This is never going to happen in the test, because the main loop is no longer running at that point. I don't think it's worth it to consider the database open while main thread callbacks are pending, they are just notification and the client is no longer insterested on tehm afer closing the database. I think it's bettter and simpler to simple cancel the pending callbacks on database close. That ensures that isOpen() after close() is always false.
Attachments
Patch
(11.68 KB, patch)
2017-08-18 06:15 PDT
,
Carlos Garcia Campos
mcatanzaro
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1
2017-08-18 06:15:57 PDT
Created
attachment 318492
[details]
Patch
Build Bot
Comment 2
2017-08-18 06:17:49 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See
http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Michael Catanzaro
Comment 3
2017-08-18 09:23:13 PDT
Comment on
attachment 318492
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=318492&action=review
Nice!
> Source/WebKit/UIProcess/API/glib/IconDatabase.h:196 > + if (!m_timer.isActive())
I would add a blank line here.
> Source/WebKit/UIProcess/API/glib/IconDatabase.h:229 > + RunLoop::Timer<MainThreadNotifier> m_timer;
You're certain m_timer does not need to be guarded by a lock? Why not?
Carlos Garcia Campos
Comment 4
2017-08-19 00:52:57 PDT
(In reply to Michael Catanzaro from
comment #3
)
> Comment on
attachment 318492
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=318492&action=review
> > Nice! > > > Source/WebKit/UIProcess/API/glib/IconDatabase.h:196 > > + if (!m_timer.isActive()) > > I would add a blank line here. > > > Source/WebKit/UIProcess/API/glib/IconDatabase.h:229 > > + RunLoop::Timer<MainThreadNotifier> m_timer; > > You're certain m_timer does not need to be guarded by a lock? Why not?
Yes, because it's thread-safe
Carlos Garcia Campos
Comment 5
2017-08-28 04:18:35 PDT
Committed
r221238
: <
http://trac.webkit.org/changeset/221238
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug