diff options
| -rw-r--r-- | bus/activation.c | 83 | ||||
| -rw-r--r-- | configure.in | 1 | ||||
| -rw-r--r-- | test/data/valid-service-files/org.freedesktop.DBus.TestSuiteForkingEchoService.service.in | 3 | ||||
| -rw-r--r-- | test/name-test/Makefile.am | 2 | ||||
| -rwxr-xr-x | test/name-test/run-test.sh | 6 | ||||
| -rw-r--r-- | test/name-test/test-activation-forking.py | 60 | ||||
| -rw-r--r-- | test/test-service.c | 32 |
7 files changed, 155 insertions, 32 deletions
diff --git a/bus/activation.c b/bus/activation.c index 782ffed8..00caac27 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -1212,8 +1212,8 @@ pending_activation_failed (BusPendingActivation *pending_activation, * Depending on the exit code of the helper, set the error accordingly */ static void -handle_activation_exit_error (int exit_code, - DBusError *error) +handle_servicehelper_exit_error (int exit_code, + DBusError *error) { switch (exit_code) { @@ -1268,13 +1268,24 @@ babysitter_watch_callback (DBusWatch *watch, BusPendingActivation *pending_activation = data; dbus_bool_t retval; DBusBabysitter *babysitter; + dbus_bool_t uses_servicehelper; babysitter = pending_activation->babysitter; - + _dbus_babysitter_ref (babysitter); - + retval = dbus_watch_handle (watch, condition); + /* There are two major cases here; are we the system bus or the session? Here this + * is distinguished by whether or not we use a setuid helper launcher. With the launch helper, + * some process exit codes are meaningful, processed by handle_servicehelper_exit_error. + * + * In both cases though, just ignore when a process exits with status 0; it's possible for + * a program to (misguidedly) "daemonize", and that appears to us as an exit. This closes a race + * condition between this code and the child process claiming the bus name. + */ + uses_servicehelper = bus_context_get_servicehelper (pending_activation->activation->context) != NULL; + /* FIXME this is broken in the same way that * connection watches used to be; there should be * a separate callback for status change, instead @@ -1284,43 +1295,59 @@ babysitter_watch_callback (DBusWatch *watch, * Fixing this lets us move dbus_watch_handle * calls into dbus-mainloop.c */ - if (_dbus_babysitter_get_child_exited (babysitter)) { DBusError error; DBusHashIter iter; - + dbus_bool_t activation_failed; + int exit_code = 0; + dbus_error_init (&error); + _dbus_babysitter_set_child_exit_error (babysitter, &error); - /* refine the error code if we got an exit code */ - if (dbus_error_has_name (&error, DBUS_ERROR_SPAWN_CHILD_EXITED)) - { - int exit_code = 0; - if (_dbus_babysitter_get_child_exit_status (babysitter, &exit_code)) + /* Explicitly check for SPAWN_CHILD_EXITED to avoid overwriting an + * exec error */ + if (dbus_error_has_name (&error, DBUS_ERROR_SPAWN_CHILD_EXITED) + && _dbus_babysitter_get_child_exit_status (babysitter, &exit_code)) + { + activation_failed = exit_code != 0; + + dbus_error_free(&error); + + if (activation_failed) { - dbus_error_free (&error); - handle_activation_exit_error (exit_code, &error); + if (uses_servicehelper) + handle_servicehelper_exit_error (exit_code, &error); + else + _dbus_babysitter_set_child_exit_error (babysitter, &error); } - } - - /* Destroy all pending activations with the same exec */ - _dbus_hash_iter_init (pending_activation->activation->pending_activations, - &iter); - while (_dbus_hash_iter_next (&iter)) + } + else { - BusPendingActivation *p = _dbus_hash_iter_get_value (&iter); - - if (p != pending_activation && strcmp (p->exec, pending_activation->exec) == 0) - pending_activation_failed (p, &error); + activation_failed = TRUE; } - - /* Destroys the pending activation */ - pending_activation_failed (pending_activation, &error); - dbus_error_free (&error); + if (activation_failed) + { + /* Destroy all pending activations with the same exec */ + _dbus_hash_iter_init (pending_activation->activation->pending_activations, + &iter); + while (_dbus_hash_iter_next (&iter)) + { + BusPendingActivation *p = _dbus_hash_iter_get_value (&iter); + + if (p != pending_activation && strcmp (p->exec, pending_activation->exec) == 0) + pending_activation_failed (p, &error); + } + + /* Destroys the pending activation */ + pending_activation_failed (pending_activation, &error); + + dbus_error_free (&error); + } } - + _dbus_babysitter_unref (babysitter); return retval; diff --git a/configure.in b/configure.in index 2e6422c1..7feb1a93 100644 --- a/configure.in +++ b/configure.in @@ -1441,6 +1441,7 @@ test/data/valid-config-files-system/debug-allow-all-pass.conf test/data/valid-config-files-system/debug-allow-all-fail.conf test/data/valid-service-files/org.freedesktop.DBus.TestSuite.PrivServer.service test/data/valid-service-files/org.freedesktop.DBus.TestSuiteEchoService.service +test/data/valid-service-files/org.freedesktop.DBus.TestSuiteForkingEchoService.service test/data/valid-service-files/org.freedesktop.DBus.TestSuiteSegfaultService.service test/data/valid-service-files/org.freedesktop.DBus.TestSuiteShellEchoServiceSuccess.service test/data/valid-service-files/org.freedesktop.DBus.TestSuiteShellEchoServiceFail.service diff --git a/test/data/valid-service-files/org.freedesktop.DBus.TestSuiteForkingEchoService.service.in b/test/data/valid-service-files/org.freedesktop.DBus.TestSuiteForkingEchoService.service.in new file mode 100644 index 00000000..49fcac39 --- /dev/null +++ b/test/data/valid-service-files/org.freedesktop.DBus.TestSuiteForkingEchoService.service.in @@ -0,0 +1,3 @@ +[D-BUS Service] +Name=org.freedesktop.DBus.TestSuiteForkingEchoService +Exec=@TEST_SERVICE_BINARY@ org.freedesktop.DBus.TestSuiteForkingEchoService fork diff --git a/test/name-test/Makefile.am b/test/name-test/Makefile.am index 1c73b877..d8e72d14 100644 --- a/test/name-test/Makefile.am +++ b/test/name-test/Makefile.am @@ -10,7 +10,7 @@ else TESTS= endif -EXTRA_DIST=run-test.sh run-test-systemserver.sh test-wait-for-echo.py +EXTRA_DIST=run-test.sh run-test-systemserver.sh test-wait-for-echo.py test-activation-forking.py if DBUS_BUILD_TESTS diff --git a/test/name-test/run-test.sh b/test/name-test/run-test.sh index fba45584..4eb24252 100755 --- a/test/name-test/run-test.sh +++ b/test/name-test/run-test.sh @@ -50,3 +50,9 @@ ${DBUS_TOP_BUILDDIR}/libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/name- echo "running test-shutdown" ${DBUS_TOP_BUILDDIR}/libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/name-test/test-shutdown || die "test-shutdown failed" + +echo "running test activation forking" +if ! python $DBUS_TOP_SRCDIR/test/name-test/test-activation-forking.py; then + echo "Failed test-activation-forking" + exit 1 +fi diff --git a/test/name-test/test-activation-forking.py b/test/name-test/test-activation-forking.py new file mode 100644 index 00000000..0d820754 --- /dev/null +++ b/test/name-test/test-activation-forking.py @@ -0,0 +1,60 @@ +#!/usr/bin/env python + +import os,sys + +try: + import gobject + import dbus + import dbus.mainloop.glib +except: + print "Failed import, aborting test" + sys.exit(0) + +dbus.mainloop.glib.DBusGMainLoop(set_as_default=True) +loop = gobject.MainLoop() + +exitcode = 0 + +bus = dbus.SessionBus() +bus_iface = dbus.Interface(bus.get_object('org.freedesktop.DBus', '/org/freedesktop/DBus'), 'org.freedesktop.DBus') + +o = bus.get_object('org.freedesktop.DBus.TestSuiteForkingEchoService', '/org/freedesktop/TestSuite') +i = dbus.Interface(o, 'org.freedesktop.TestSuite') + +# Start it up +reply = i.Echo("hello world") +print "TestSuiteForkingEchoService initial reply OK" + +def ignore(*args, **kwargs): + pass + +# Now monitor for exits, when that happens, start it up again. +# The goal here is to try to hit any race conditions in activation. +counter = 0 +def on_forking_echo_owner_changed(name, old, new): + global counter + global o + global i + if counter > 10: + print "Activated 10 times OK, TestSuiteForkingEchoService pass" + loop.quit() + return + counter += 1 + if new == '': + o = bus.get_object('org.freedesktop.DBus.TestSuiteForkingEchoService', '/org/freedesktop/TestSuite') + i = dbus.Interface(o, 'org.freedesktop.TestSuite') + i.Echo("counter %r" % counter) + i.Exit(reply_handler=ignore, error_handler=ignore) + +bus_iface.connect_to_signal('NameOwnerChanged', on_forking_echo_owner_changed, arg0='org.freedesktop.DBus.TestSuiteForkingEchoService') + +i.Exit(reply_handler=ignore, error_handler=ignore) + +def check_counter(): + if counter == 0: + print "Failed to get NameOwnerChanged for TestSuiteForkingEchoService" + sys.exit(1) +gobject.timeout_add(15000, check_counter) + +loop.run() +sys.exit(0) diff --git a/test/test-service.c b/test/test-service.c index ee0086ca..9a129aac 100644 --- a/test/test-service.c +++ b/test/test-service.c @@ -396,7 +396,33 @@ main (int argc, DBusError error; int result; DBusConnection *connection; - + const char *name; + dbus_bool_t do_fork; + + if (argc != 3) + { + name = "org.freedesktop.DBus.TestSuiteEchoService"; + do_fork = FALSE; + } + else + { + name = argv[1]; + do_fork = strcmp (argv[2], "fork") == 0; + } + + /* The bare minimum for simulating a program "daemonizing"; the intent + * is to test services which move from being legacy init scripts to + * activated services. + * https://bugzilla.redhat.com/show_bug.cgi?id=545267 + */ + if (do_fork) + { + pid_t pid = fork (); + if (pid != 0) + exit (0); + sleep (1); + } + dbus_error_init (&error); connection = dbus_bus_get (DBUS_BUS_STARTER, &error); if (connection == NULL) @@ -431,8 +457,8 @@ main (int argc, if (d != (void*) 0xdeadbeef) die ("dbus_connection_get_object_path_data() doesn't seem to work right\n"); } - - result = dbus_bus_request_name (connection, "org.freedesktop.DBus.TestSuiteEchoService", + + result = dbus_bus_request_name (connection, name, 0, &error); if (dbus_error_is_set (&error)) { |