DBus Test Runner

Merge lp:~mhr3/dbus-test-runner/watch-pipes-for-hup into lp:dbus-test-runner/13.10

  1. watch-pipes-for-hup
  2. Merge into trunk.13.10
Proposed by Michal Hruby
Status: Merged
Approved by: Ted Gould
Approved revision: 75
Merged at revision: 65
Proposed branch: lp:~mhr3/dbus-test-runner/watch-pipes-for-hup
Merge into: lp:dbus-test-runner/13.10
Diff against target: 235 lines (+56/-20)
5 files modified
libdbustest/Makefile.am (+2/-0)
libdbustest/bustle.c (+1/-1)
libdbustest/leash.c (+12/-11)
libdbustest/process.c (+13/-5)
libdbustest/service.c (+28/-3)
To merge this branch: bzr merge lp:~mhr3/dbus-test-runner/watch-pipes-for-hup
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Ted Gould (community) Approve
Alberto Mardegan (community) Approve
Review via email: mp+176092@code.launchpad.net

Commit message

Watch created pipes for the G_IO_HUP event, as when a pipe closes, polling it will return this event.

Description of the change

Watch created pipes for the G_IO_HUP event, as when a pipe closes, polling it will return this event.

Without this patch, the test runner is somewhat racy as it expects to always get a child watch event before a pipe closing. Moreover not handling HUP can cause 100% CPU usage, since polling a closed pipe will keep returning POLLHUP and nothing in the code removed the pipe from the poll fds.

Also clean up the code in the watchdog process, use g_unix_signal_add, which plays nicer with glib's main loop.

Lastly, don't print empty line after each received line from the process tasks. :)

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:67
http://jenkins.qa.ubuntu.com/job/dbus-test-runner-ci/3/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-amd64-ci/3/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-armhf-ci/3/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-amd64-ci/3/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-armhf-ci/3/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/dbus-test-runner-ci/3/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:68
http://jenkins.qa.ubuntu.com/job/dbus-test-runner-ci/4/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-amd64-ci/4
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-armhf-ci/4
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-amd64-ci/4
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-armhf-ci/4

Click here to trigger a rebuild:
http://s-jenkins:8080/job/dbus-test-runner-ci/4/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alberto Mardegan (mardy) wrote :

116 + message = g_strdup_printf("Exitted with status %d", status);

"Exited"

Other than that, I tested your changes against one of my projects and it works just fine.
And the removal of the blank lines is invaluable, thanks! :-)

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:69
http://jenkins.qa.ubuntu.com/job/dbus-test-runner-ci/5/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-amd64-ci/5/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-armhf-ci/5
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-amd64-ci/5
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-armhf-ci/5

Click here to trigger a rebuild:
http://s-jenkins:8080/job/dbus-test-runner-ci/5/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Alberto Mardegan (mardy) wrote :

LGTM!

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:69
http://jenkins.qa.ubuntu.com/job/dbus-test-runner-ci/6/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-amd64-ci/6/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-armhf-ci/6
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-amd64-ci/6
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-armhf-ci/6

Click here to trigger a rebuild:
http://s-jenkins:8080/job/dbus-test-runner-ci/6/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:71
http://jenkins.qa.ubuntu.com/job/dbus-test-runner-ci/7/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-amd64-ci/7/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-armhf-ci/7
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-amd64-ci/7
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-armhf-ci/7

Click here to trigger a rebuild:
http://s-jenkins:8080/job/dbus-test-runner-ci/7/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:72
http://jenkins.qa.ubuntu.com/job/dbus-test-runner-ci/8/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-amd64-ci/8
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-armhf-ci/8
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-amd64-ci/8
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-armhf-ci/8

Click here to trigger a rebuild:
http://s-jenkins:8080/job/dbus-test-runner-ci/8/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:73
http://jenkins.qa.ubuntu.com/job/dbus-test-runner-ci/9/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-amd64-ci/9/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-armhf-ci/9
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-amd64-ci/9
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-armhf-ci/9

Click here to trigger a rebuild:
http://s-jenkins:8080/job/dbus-test-runner-ci/9/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:74
http://jenkins.qa.ubuntu.com/job/dbus-test-runner-ci/10/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-amd64-ci/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-armhf-ci/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-amd64-ci/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-armhf-ci/10

Click here to trigger a rebuild:
http://s-jenkins:8080/job/dbus-test-runner-ci/10/rebuild

review: Approve (continuous-integration)
Revision history for this message
Ted Gould (ted) :
review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
Unapproved changes made after approval.
http://jenkins.qa.ubuntu.com/job/dbus-test-runner-autolanding/2/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-amd64-autolanding/2
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-quantal-armhf-autolanding/2
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-amd64-autolanding/2
    SUCCESS: http://jenkins.qa.ubuntu.com/job/dbus-test-runner-saucy-armhf-autolanding/2

review: Needs Fixing (continuous-integration)
Revision history for this message
Ted Gould (ted) wrote :

Nice job Jenkins bot!

Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'libdbustest/Makefile.am'
2--- libdbustest/Makefile.am 2013-01-29 16:24:04 +0000
3+++ libdbustest/Makefile.am 2013-07-22 18:03:24 +0000
4@@ -4,6 +4,8 @@
5 ABI_VERSION = 1
6 API_VERSION = 1
7
8+AM_CFLAGS = -Wall
9+
10 libdbustestincludedir=$(includedir)/libdbustest-$(API_VERSION)/libdbustest
11 libdbustestinclude_HEADERS = \
12 bustle.h \
13
14=== modified file 'libdbustest/bustle.c'
15--- libdbustest/bustle.c 2013-02-21 17:33:04 +0000
16+++ libdbustest/bustle.c 2013-07-22 18:03:24 +0000
17@@ -271,7 +271,7 @@
18
19 bustler->priv->stderr = g_io_channel_unix_new(bustle_stderr_num);
20 g_io_add_watch(bustler->priv->stderr,
21- G_IO_IN, /* conditions */
22+ G_IO_IN | G_IO_HUP | G_IO_ERR, /* conditions */
23 bustle_write_error, /* func */
24 bustler); /* func data */
25
26
27=== modified file 'libdbustest/leash.c'
28--- libdbustest/leash.c 2013-01-29 20:32:11 +0000
29+++ libdbustest/leash.c 2013-07-22 18:03:24 +0000
30@@ -18,12 +18,13 @@
31 */
32
33 #include <glib.h>
34+#include <glib-unix.h>
35
36 GMainLoop * mainloop = NULL;
37-gulong timer = 0;
38+guint timer = 0;
39 pid_t victim = 0;
40
41-static void sigterm_graceful_exit (int signal);
42+static gboolean sigterm_graceful_exit (gpointer user_data);
43
44 static gboolean
45 destroy_everyone (gpointer user_data)
46@@ -33,7 +34,7 @@
47 }
48
49 sigterm_graceful_exit(0);
50- return;
51+ return FALSE;
52 }
53
54 static void
55@@ -48,18 +49,18 @@
56 return;
57 }
58
59-static void
60-sighup_dont_die (int signal)
61+static gboolean
62+sighup_dont_die (gpointer user_data)
63 {
64 restart_handler();
65- return;
66+ return TRUE;
67 }
68
69-static void
70-sigterm_graceful_exit (int signal)
71+static gboolean
72+sigterm_graceful_exit (gpointer user_data)
73 {
74 g_main_loop_quit(mainloop);
75- return;
76+ return FALSE;
77 }
78
79 int
80@@ -74,8 +75,8 @@
81 g_type_init();
82 #endif
83
84- signal(SIGTERM, sigterm_graceful_exit);
85- signal(SIGHUP, sighup_dont_die);
86+ g_unix_signal_add (SIGTERM, sigterm_graceful_exit, NULL);
87+ g_unix_signal_add (SIGHUP, sighup_dont_die, NULL);
88
89 victim = atoi(argv[1]);
90
91
92=== modified file 'libdbustest/process.c'
93--- libdbustest/process.c 2013-02-21 14:24:32 +0000
94+++ libdbustest/process.c 2013-07-22 18:03:24 +0000
95@@ -120,6 +120,7 @@
96 continue;
97 }
98
99+ line[termloc] = '\0';
100 dbus_test_task_print(DBUS_TEST_TASK(process), line);
101 g_free(line);
102 }
103@@ -150,6 +151,7 @@
104 static void
105 proc_watcher (GPid pid, gint status, gpointer data)
106 {
107+ gchar *message;
108 g_return_if_fail(DBUS_TEST_IS_PROCESS(data));
109 DbusTestProcess * process = DBUS_TEST_PROCESS(data);
110
111@@ -161,6 +163,12 @@
112 process->priv->complete = TRUE;
113 process->priv->status = status;
114
115+ if (status) {
116+ message = g_strdup_printf("Exited with status %d", status);
117+ dbus_test_task_print(DBUS_TEST_TASK(process), message);
118+ g_free(message);
119+ }
120+
121 g_signal_emit_by_name(G_OBJECT(process), DBUS_TEST_TASK_SIGNAL_STATE_CHANGED, DBUS_TEST_TASK_STATE_FINISHED, NULL);
122
123 return;
124@@ -188,15 +196,15 @@
125 continue;
126 }
127
128+ line[termloc] = '\0';
129 dbus_test_task_print(DBUS_TEST_TASK(process), line);
130 g_free(line);
131 } while (G_IO_IN & g_io_channel_get_buffer_condition(channel));
132
133 if (done) {
134- process->priv->complete = TRUE;
135- process->priv->status = -1;
136-
137- g_signal_emit_by_name(G_OBJECT(process), DBUS_TEST_TASK_SIGNAL_STATE_CHANGED, DBUS_TEST_TASK_STATE_FINISHED, NULL);
138+ process->priv->io_watch = 0;
139+ // wait for proc_watcher to switch state to FINISHED
140+ return FALSE;
141 }
142
143 return TRUE;
144@@ -249,7 +257,7 @@
145 process->priv->io_chan = g_io_channel_unix_new(proc_stdout);
146 g_io_channel_set_buffer_size(process->priv->io_chan, 10 * 1024 * 1024); /* 10 MB should be enough for anyone */
147 process->priv->io_watch = g_io_add_watch(process->priv->io_chan,
148- G_IO_IN, /* conditions */
149+ G_IO_IN | G_IO_HUP | G_IO_ERR, /* conditions */
150 proc_writes, /* func */
151 process); /* func data */
152
153
154=== modified file 'libdbustest/service.c'
155--- libdbustest/service.c 2013-07-15 13:33:26 +0000
156+++ libdbustest/service.c 2013-07-22 18:03:24 +0000
157@@ -22,8 +22,10 @@
158 #endif
159
160 #include <unistd.h>
161+#include <string.h>
162
163 #include <glib.h>
164+#include <gio/gio.h>
165 #include "glib-compat.h"
166
167 #include "dbus-test.h"
168@@ -34,6 +36,7 @@
169 STATE_INIT,
170 STATE_DAEMON_STARTING,
171 STATE_DAEMON_STARTED,
172+ STATE_DAEMON_FAILED,
173 STATE_STARTING,
174 STATE_STARTED,
175 STATE_RUNNING,
176@@ -427,6 +430,7 @@
177
178 if (error != NULL) {
179 g_critical("Unable to start dbus daemon: %s", error->message);
180+ g_error_free(error);
181 service->priv->daemon_crashed = TRUE;
182 return;
183 }
184@@ -437,13 +441,33 @@
185
186 service->priv->dbus_io = g_io_channel_unix_new(dbus_stdout);
187 service->priv->dbus_io_watch = g_io_add_watch(service->priv->dbus_io,
188- G_IO_IN | G_IO_ERR, /* conditions */
189+ G_IO_IN | G_IO_HUP | G_IO_ERR, /* conditions */
190 dbus_writes, /* func */
191 service); /* func data */
192
193 g_main_loop_run(service->priv->mainloop);
194+
195+ /* we should have a usable connection now, let's check */
196+ gchar **tokens = g_strsplit (g_getenv ("DBUS_SESSION_BUS_ADDRESS"),
197+ ",", 0);
198+ guint i;
199+ gboolean is_valid = FALSE;
200+ for (i = 0; i < g_strv_length (tokens); i++) {
201+ if (strlen (tokens[i]) && g_dbus_is_supported_address (tokens[i], NULL)) {
202+ is_valid = TRUE;
203+ break;
204+ }
205+ }
206+ g_strfreev(tokens);
207+
208+ if (!is_valid) {
209+ service->priv->state = STATE_DAEMON_FAILED;
210+ g_critical ("DBus daemon failed: Bus address is not supported");
211+ g_error_free (error);
212+ return;
213+ }
214+
215 service->priv->state = STATE_DAEMON_STARTED;
216-
217 return;
218 }
219
220@@ -454,6 +478,7 @@
221
222 start_daemon(service);
223 g_return_if_fail(g_getenv("DBUS_SESSION_BUS_ADDRESS") != NULL);
224+ g_return_if_fail(service->priv->state != STATE_DAEMON_FAILED);
225
226 if (all_tasks(service, all_tasks_started_helper)) {
227 /* If we have all started we can mark it as such as long
228@@ -507,7 +532,7 @@
229 static int
230 get_status (DbusTestService * service)
231 {
232- if (service->priv->daemon_crashed) {
233+ if (service->priv->daemon_crashed || service->priv->state == STATE_DAEMON_FAILED) {
234 return -1;
235 }
236

Subscribers

People subscribed via source and target branches