Merge lp:~larsu/dbus-test-runner/fix-test-race into lp:dbus-test-runner/15.10

Proposed by Lars Karlitski
Status: Merged
Approved by: Iain Lane
Approved revision: 96
Merged at revision: 96
Proposed branch: lp:~larsu/dbus-test-runner/fix-test-race
Merge into: lp:dbus-test-runner/15.10
Diff against target: 65 lines (+22/-3)
2 files modified
tests/Makefile.am (+2/-2)
tests/test-own-name.c (+20/-1)
To merge this branch: bzr merge lp:~larsu/dbus-test-runner/fix-test-race
Reviewer Review Type Date Requested Status
Iain Lane Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+270488@code.launchpad.net

Commit message

test-param-wait-system: fix race between tasks

Calling RequestName with gdbus-tool to signal the tested binary that a name is owned is racy: since the name is only owned very shortly, the watch might not have been established before the name is already gone again.

Fix this by holding on to the name a bit longer with the test-own-name helper (add a --system flag to that). A better fix would be to only start the second task when the watch has been established. dbus-test-runner always starts tasks in parallel though and changing this would further complicate its command line interface.

Description of the change

test-param-wait-system: fix race between tasks

Calling RequestName with gdbus-tool to signal the tested binary that a name is owned is racy: since the name is only owned very shortly, the watch might not have been established before the name is already gone again.

Fix this by holding on to the name a bit longer with the test-own-name helper (add a --system flag to that). A better fix would be to only start the second task when the watch has been established. dbus-test-runner always starts tasks in parallel though and changing this would further complicate its command line interface.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Iain Lane (laney) wrote :

thanks, let's upload this!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/Makefile.am'
2--- tests/Makefile.am 2015-01-30 17:10:47 +0000
3+++ tests/Makefile.am 2015-09-09 07:37:42 +0000
4@@ -121,9 +121,9 @@
5 XFAIL_TESTS += test-param-only-wait
6
7 TESTS += test-param-wait-system
8-test-param-wait-system: Makefile.am
9+test-param-wait-system: Makefile.am test-own-name
10 @echo "#!/bin/sh" > $@
11- @echo $(DBUS_RUNNER) --bus-type=system --task ls --task-bus=system --wait-for org.test.test --task gdbus --parameter call --parameter --system --parameter --dest --parameter org.freedesktop.DBus --parameter --object-path --parameter / --parameter --method --parameter org.freedesktop.DBus.RequestName --parameter org.test.test --parameter 0 --task-bus=system >> $@
12+ @echo $(DBUS_RUNNER) --bus-type=system --task ls --task-bus=system --wait-for org.test.test --task $(builddir)/test-own-name --parameter --system --parameter org.test.test --ignore-return --task-bus=system >> $@
13 @chmod +x $@
14
15 TESTS += test-param-multi-wait
16
17=== modified file 'tests/test-own-name.c'
18--- tests/test-own-name.c 2013-01-29 20:32:11 +0000
19+++ tests/test-own-name.c 2015-09-09 07:37:42 +0000
20@@ -18,10 +18,27 @@
21 int
22 main (int argc, char * argv[])
23 {
24+ gboolean system_bus = FALSE;
25+ GOptionContext *options;
26+ GError *error = NULL;
27+
28+ const GOptionEntry option_entries[] = {
29+ { "system", 'y', G_OPTION_FLAG_NONE, G_OPTION_ARG_NONE, &system_bus, "Own the name on the system bus", NULL },
30+ { NULL }
31+ };
32+
33 #ifndef GLIB_VERSION_2_36
34 g_type_init();
35 #endif
36
37+ options = g_option_context_new(NULL);
38+ g_option_context_add_main_entries(options, option_entries, NULL);
39+ if (!g_option_context_parse(options, &argc, &argv, &error)) {
40+ g_printerr("%s", error->message);
41+ g_error_free(error);
42+ return 1;
43+ }
44+
45 if (argc != 2) {
46 g_error("ARG, need a single argument");
47 return 1;
48@@ -29,7 +46,7 @@
49
50 g_debug("Trying for name: %s", argv[1]);
51
52- g_bus_own_name(G_BUS_TYPE_SESSION,
53+ g_bus_own_name(system_bus ? G_BUS_TYPE_SYSTEM : G_BUS_TYPE_SESSION,
54 argv[1],
55 G_BUS_NAME_OWNER_FLAGS_NONE,
56 NULL, /* bus aquired */
57@@ -43,7 +60,9 @@
58 g_timeout_add_seconds(2, end_of_line, mainloop);
59
60 g_main_loop_run(mainloop);
61+
62 g_main_loop_unref(mainloop);
63+ g_option_context_free(options);
64
65 g_debug("Quitting");
66

Subscribers

People subscribed via source and target branches