[Webkit-unassigned] [Bug 161481] [GTK] Use GTestDBus instead of dbus-launch in WebKitTestBus.cpp

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Nov 2 05:52:30 PDT 2016


https://bugs.webkit.org/show_bug.cgi?id=161481

--- Comment #13 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #12)
> (In reply to comment #10)
> > Why? Could you show a bt of one of such crashes? Because our current code is
> > calling setenv in the same place, this patch is only replacing the code that
> > GTestDBus with GTestDBus.
> 
> Here's one example of such a crash, with a backtrace:
> 
> https://bugzilla.gnome.org/show_bug.cgi?id=754951
> 
> We had a second case in some important GNOME component recently, but I
> honestly don't remember where and can't find it now. And we have third one
> in Endless's private bug tracker right now.

I don't care about other applications crashing, this bug is about our unit tests, we have done this forever and I've never seen a crash, so I don't understand why you say that this patch is going to be the source of crashes and flaky tests.

> > > We need to be careful to only call
> > > such functions (a) at the very top of main(),
> > 
> > Which is exactly what happens. This is not public API, this is an internal
> > helper class designed to be used by tests that should do this at the very
> > beginning of beforeAll():
> > 
> > bus = new WebKitTestBus();
> > if (!bus->run())
> >     return;
> > 
> > And beforeAll() is called by main() right after setting up the environment
> > and registering gresources. There isn't any other code before that, nor any
> > other secondary thread created.
> 
> OK, then it should be OK, but it is so easy to misuse that I would prefer
> that you add an assertion to guarantee this. Unfortunately the best example
> I found requires reading /proc:
> 
> http://stackoverflow.com/a/13993822/1120203
> 
> but it could be done inside #ifndef NDEBUG. Then if that check passes, we
> can be confident that GLib isn't creating any secondary thread somewhere
> before the setenv.

This is private API for unit tests, I don't think we need to complicate things to fix a problem that doesn't exist.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20161102/275abf09/attachment.html>


More information about the webkit-unassigned mailing list