Merge lp:~jamesodhunt/upstart/bug-1258098 into lp:upstart

[James Hunt]

[Dimitri John Ledkov (xnox)]

The code looks ok, the test is not passing for me:

not ok 5 - ensure Session Init retains D-Bus address across a re-exec
 wrong value for running, expected 1 got 0
 at test_util_common.c:132 (wait_for_upstart).
Aborted (core dumped)

Also note: https://bugs.launchpad.net/upstart/+bug/1288243

review: Needs Information

=== modified file 'ChangeLog'
--- ChangeLog	2014-01-17 15:39:55 +0000
+++ ChangeLog	2014-01-21 13:39:11 +0000
@@ -1,3 +1,4 @@
+<<<<<<< TREE
 2014-01-17  James Hunt  <[e-mail]>
 
 	* init/conf.c:
@@ -64,6 +65,20 @@
 	* init/man/init.5: Provide more detail on setuid and setgid stanzas
 	  (debian bug#732127).
 
+=======
[phone]  James Hunt  <[e-mail]>
+
+	* init/control.c:
+	  - control_serialise_bus_address(), control_deserialise_bus_address():
+	    New functions to handle carrying the control bus address across a
+	    re-exec (LP: #1258098).
+	* init/state.c:
+	  - state_to_string(): Handle control bus address serialisation.
+	  - state_from_string(): Handle control bus address deserialisation.
+	* util/tests/test_initctl.c: test_dbus_connection(): New test:
+	  - "ensure Session Init retains D-Bus address across a re-exec"
+
+>>>>>>> MERGE-SOURCE
 2013-11-23  Steve Langasek  <[e-mail]>
 
 	* init/tests/test_state.c: fix test case to not assume SIGUSR1 == 10;

=== modified file 'init/control.c'
--- init/control.c	2013-11-05 16:15:19 +0000
+++ init/control.c	2014-01-21 13:39:11 +0000
@@ -1887,3 +1887,66 @@
 
 	return 0;
 }
+
+/**
+ * control_serialise_bus_address:
+ *
+ * Convert control_bus_address into JSON representation.
+ *
+ * Returns: JSON string representing control_bus_address or NULL if
+ * control_bus_address not set or on error.
+ *
+ * Note: If NULL is returned, check the value of control_bus_address
+ * itself to determine if the error is real.
+ **/
+json_object *
+control_serialise_bus_address (void)
+{
+	control_init ();
+
+	/* A NULL return represents a JSON null */
+	return control_bus_address
+		? json_object_new_string (control_bus_address)
+		: NULL;
+}
+
+/**
+ * control_deserialise_bus_address:
+ *
+ * @json: root of JSON-serialised state.
+ *
+ * Convert JSON representation of control_bus_address back into a native
+ * string.
+ *
+ * Returns: 0 on success, -1 on error.
+ **/
+int
+control_deserialise_bus_address (json_object *json)
+{
+	const char  *address;
+
+	nih_assert (json);
+	nih_assert (! control_bus_address);
+
+	control_init ();
+
+	/* control_bus_address was never set */
+	if (state_check_json_type (json, null))
+		return 0;
+
+	if (! state_check_json_type (json, string))
+		goto error;
+
+	address = json_object_get_string (json);
+	if (! address)
+		goto error;
+
+	control_bus_address = nih_strdup (NULL, address);
+	if (! control_bus_address)
+		goto error;
+
+	return 0;
+
+error:
+	return -1;
+}

=== modified file 'init/control.h'
--- init/control.h	2013-10-25 13:49:49 +0000
+++ init/control.h	2014-01-21 13:39:11 +0000
@@ -142,8 +142,7 @@
 int control_conn_to_index (const DBusConnection *connection)
 	__attribute__ ((warn_unused_result));
 
-DBusConnection *
-control_conn_from_index (int conn_index)
+DBusConnection *control_conn_from_index (int conn_index)
 	__attribute__ ((warn_unused_result));
 
 int control_bus_release_name (void)
@@ -184,26 +183,29 @@
 		 char            **value)
 	__attribute__ ((warn_unused_result));
 
-int
-control_list_env (void             *data,
+int control_list_env (void             *data,
 		 NihDBusMessage   *message,
 		 char * const     *job_details,
 		 char           ***env)
 	__attribute__ ((warn_unused_result));
 
-int
-control_reset_env (void           *data,
+int control_reset_env (void           *data,
 		 NihDBusMessage   *message,
 		 char * const    *job_details)
 	__attribute__ ((warn_unused_result));
 
-int
-control_unset_env (void            *data,
+int control_unset_env (void            *data,
 		   NihDBusMessage  *message,
 		   char * const    *job_details,
 		   const char      *name)
 	__attribute__ ((warn_unused_result));
 
+json_object *control_serialise_bus_address (void)
+	__attribute__ ((warn_unused_result));
+
+int control_deserialise_bus_address (json_object *json)
+	__attribute__ ((warn_unused_result));
+
 NIH_END_EXTERN
 
 #endif /* INIT_CONTROL_H */

=== modified file 'init/state.c'
--- init/state.c	2013-10-11 13:35:51 +0000
+++ init/state.c	2014-01-21 13:39:11 +0000
@@ -54,6 +54,7 @@
 json_object *json_conf_sources = NULL;
 
 extern char *log_dir;
+extern char *control_bus_address;
 
 /**
  * args_copy:
@@ -340,6 +341,7 @@
 {
 	json_object  *json;
 	json_object  *json_job_environ;
+	json_object  *json_control_bus_address;
 	const char   *value;
 
 	nih_assert (json_string);
@@ -366,6 +368,20 @@
 
 	json_object_object_add (json, "events", json_events);
 
+	json_control_bus_address = control_serialise_bus_address ();
+
+	/* Take care to distinguish between memory failure and an
+	 * as-yet-not-set control bus address.
+	 */
+	if (! json_control_bus_address && control_bus_address) {
+		nih_error ("%s %s",
+				_("Failed to serialise"),
+			       _("control bus address"));
+		goto error;
+	}
+
+	json_object_object_add (json, "control_bus_address", json_control_bus_address);
+
 	json_job_environ = job_class_serialise_job_environ ();
 
 	if (! json_job_environ) {
@@ -427,6 +443,7 @@
 	int                       ret = -1;
 	json_object              *json;
 	json_object              *json_job_environ;
+	json_object              *json_control_bus_address;
 	enum json_tokener_error   error;
 
 	nih_assert (state);
@@ -458,6 +475,20 @@
 		goto out;
 	}
 
+	ret = json_object_object_get_ex (json, "control_bus_address", &json_control_bus_address);
+
+	if (json_control_bus_address) {
+		if (control_deserialise_bus_address (json_control_bus_address) < 0) {
+			nih_error ("%s control details", _("Failed to deserialise"));
+			goto out;
+		}
+	} else if (! ret) {
+		/* Probably deserialising from older format that doesn't
+		 * encode control details.
+		 */
+		nih_warn ("%s", _("No control details present in state data"));
+	}
+
 	/* Again, we cannot error here since older JSON state data did
 	 * not encode ConfSource or ConfFile objects.
 	 */

=== modified file 'util/tests/test_initctl.c'
--- util/tests/test_initctl.c	2013-11-13 02:50:36 +0000
+++ util/tests/test_initctl.c	2014-01-21 13:39:11 +0000
@@ -17052,6 +17052,27 @@
 	TEST_STR_MATCH (output[0], "init (upstart [0-9.][0-9.]*");
 	nih_free (output);
 
+	/*********************************************************************/
+	TEST_FEATURE ("ensure Session Init retains D-Bus address across a re-exec");
+
+	assert0 (unsetenv ("DBUS_SESSION_BUS_ADDRESS"));
+
+	REEXEC_UPSTART (upstart_pid, TRUE);
+
+	/* Re-apply in the test environment */
+	assert0 (setenv ("DBUS_SESSION_BUS_ADDRESS", dbus_session_address, 1));
+
+	/* It should still be possible to query the running version via
+	 * the D-Bus session bus since Upstart should have reconnected
+	 * since it was previously notified of the D-Bus address.
+	 */
+	cmd = nih_sprintf (NULL, "%s --session version 2>&1", get_initctl_binary ());
+	TEST_NE_P (cmd, NULL);
+	RUN_COMMAND (NULL, cmd, &output, &lines);
+	TEST_EQ (lines, 1);
+	TEST_STR_MATCH (output[0], "init (upstart [0-9.][0-9.]*");
+	nih_free (output);
+
 	STOP_UPSTART (upstart_pid);
 	TEST_DBUS_END (dbus_pid);