Bug 83530

Summary:	dbus-glib tests fail with modern GLib: tries to remove a GSource more than once
Product:	dbus	Reporter:	Simon McVittie <smcv>
Component:	GLib	Assignee:	Simon McVittie <smcv>
Status:	RESOLVED FIXED	QA Contact:	Rob Taylor <rob.taylor>
Severity:	blocker
Priority:	high	CC:	alban.crequy
Version:	unspecified	Keywords:	patch
Hardware:	Other
OS:	All
Whiteboard:	review?
i915 platform:		i915 features:
Attachments:	tests: only remove sources that have not already been removed

Description Simon McVittie 2014-09-05 13:48:13 UTC

Alban, I'm Cc'ing you as a reviewer because this blocks testing or merging your dbus-glib changes. I know you're not formally a dbus-glib maintainer, but nothing we do can possibly make dbus-glib all that much worse, so, whatever :-)

On a halfway modern system (Debian unstable in my case), the regression tests fail with:

Got Frobnicate signal
Calling EmitFrobnicate (3)
Got Frobnicate signal (again)
Got Frobnicate signal
Calling EmitFrobnicate (compat)
Got Frobnicate signal (compat)

(process:28582): GLib-CRITICAL **: Source ID 98 was not found when attempting to remove it
Trace/breakpoint trap (core dumped)
test-dbus-glib failed
killing message bus 28480
/home/smcv/src/fdo/dbus-glib/test/core/../../tools/run-with-tmp-session-bus.sh: script "/home/smcv/src/fdo/dbus-glib/test/core/run-test.sh" failed

Patch in a moment.

Comment 1 Simon McVittie 2014-09-05 14:07:05 UTC

Created attachment 105802 [details] [review]
tests: only remove sources that have not already been removed

With modern GLib, misusing GSource provokes a critical warning, which
is made fatal by our test framework.

It is possible (indeed likely) that some or all of the
cancel_exit_timeout() calls just before re-adding the timeout are
redundant, but I didn't want to waste time on assessing that - easier
to just do it every time.

Comment 2 Alban Crequy 2014-09-05 14:25:32 UTC

(In reply to comment #1)
> Created attachment 105802 [details] [review] [review]
> tests: only remove sources that have not already been removed
> 
> With modern GLib, misusing GSource provokes a critical warning, which
> is made fatal by our test framework.
> 
> It is possible (indeed likely) that some or all of the
> cancel_exit_timeout() calls just before re-adding the timeout are
> redundant, but I didn't want to waste time on assessing that - easier
> to just do it every time.

The patch looks good to me. And the tests pass fine with the patch.

Comment 3 Simon McVittie 2014-09-05 15:46:15 UTC

Thanks for the review, fixed in git for 0.104

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.