James Hunt has proposed merging lp:~jamesodhunt/upstart/bug-1238078 into lp:upstart.
Requested reviews: Upstart Reviewers (upstart-reviewers) Related bugs: Bug #1238078 in upstart : "global environment table is not serialised" https://bugs.launchpad.net/upstart/+bug/1238078 For more details, see: https://code.launchpad.net/~jamesodhunt/upstart/bug-1238078/+merge/190723 Fix for bug 1238078: ------------------------------------------------------------ revno: 1545 committer: James Hunt <[email protected]> branch nick: upstart-bug-1238078 timestamp: Fri 2013-10-11 17:17:19 +0100 message: * test/test_util_common.c: - session_init_reexec(): New. - set_upstart_session(): Whitespace. * test/test_util_common.h: REEXEC_UPSTART(): Update to handle Session Inits too. * util/man/initctl.8: Clarify 'set-env' behaviour. * util/tests/test_initctl.c: New tests: - "ensure 'set-env' persists across session-init re-exec". - "ensure 'set-env --global' persists across session-init re-exec". ------------------------------------------------------------ revno: 1544 fixes bug: https://launchpad.net/bugs/1238078 committer: James Hunt <[email protected]> branch nick: upstart-bug-1238078 timestamp: Fri 2013-10-11 14:35:51 +0100 message: * init/job_class.c: - job_class_serialise_job_environ(): New function to serialise global job environment table. - job_class_deserialise_job_environ(): New function to deserialise global job environment table. * init/state.c: - state_to_string(): Serialise global job environment table (LP: #1238078). - state_from_string(): Deserialise global job environment table. - _state_deserialise_str_array(): Don't attempt to free array if type check fails. -- https://code.launchpad.net/~jamesodhunt/upstart/bug-1238078/+merge/190723 Your team Upstart Reviewers is requested to review the proposed merge of lp:~jamesodhunt/upstart/bug-1238078 into lp:upstart.
=== modified file 'ChangeLog' --- ChangeLog 2013-10-04 21:34:25 +0000 +++ ChangeLog 2013-10-11 16:20:01 +0000 @@ -1,3 +1,26 @@ +2013-10-11 James Hunt <[email protected]> + + * init/job_class.c: + - job_class_serialise_job_environ(): New function to serialise global + job environment table. + - job_class_deserialise_job_environ(): New function to deserialise global + job environment table. + * init/state.c: + - state_to_string(): Serialise global job environment table + (LP: #1238078). + - state_from_string(): Deserialise global job environment table. + - _state_deserialise_str_array(): Don't attempt to free array if type + check fails. + * test/test_util_common.c: + - session_init_reexec(): New. + - set_upstart_session(): Whitespace. + * test/test_util_common.h: REEXEC_UPSTART(): Update to handle Session + Inits too. + * util/man/initctl.8: Clarify 'set-env' behaviour. + * util/tests/test_initctl.c: New tests: + - "ensure 'set-env' persists across session-init re-exec". + - "ensure 'set-env --global' persists across session-init re-exec". + 2013-01-04 Steve Langasek <[email protected] * extra/upstart-local-bridge.c: use SOCKET_PATH in our event === modified file 'init/job_class.c' --- init/job_class.c 2013-07-22 00:15:43 +0000 +++ init/job_class.c 2013-10-11 16:20:01 +0000 @@ -1802,6 +1802,58 @@ return 0; } +/** + * job_class_serialise_job_environ: + * + * Serialise the global job environment table. + * + * Returns: JSON-serialised global job environment table, or NULL on error. + **/ +json_object * +job_class_serialise_job_environ (void) +{ + json_object *json; + + job_class_environment_init (); + + json = state_serialise_str_array (job_environ); + if (! json) + goto error; + + return json; + +error: + json_object_put (json); + return NULL; +} + +/** + * job_class_deserialise_job_environ + * @json: JSON-serialised global job environment table to deserialise. + * + * Create the global job environment table from provided JSON. + * + * Returns: 0 on success, < 0 on error. + **/ +int +job_class_deserialise_job_environ (json_object *json) +{ + nih_assert (json); + + nih_assert (! job_environ); + + if (! state_check_json_type (json, array)) + goto error; + + if (! state_deserialise_str_array (NULL, json, &job_environ)) + goto error; + + return 0; + +error: + return -1; +} + /** * job_class_serialise: === modified file 'init/job_class.h' --- init/job_class.h 2013-07-21 23:54:16 +0000 +++ init/job_class.h 2013-10-11 16:20:01 +0000 @@ -360,6 +360,12 @@ JobClass * job_class_get (const char *name, Session *session) __attribute__ ((warn_unused_result)); +json_object * job_class_serialise_job_environ (void) + __attribute__ ((warn_unused_result)); + +int job_class_deserialise_job_environ (json_object *json) + __attribute__ ((warn_unused_result)); + void job_class_prepare_reexec (void); time_t job_class_max_kill_timeout (void) === modified file 'init/state.c' --- init/state.c 2013-07-15 14:54:39 +0000 +++ init/state.c 2013-10-11 16:20:01 +0000 @@ -339,6 +339,7 @@ state_to_string (char **json_string, size_t *len) { json_object *json; + json_object *json_job_environ; const char *value; nih_assert (json_string); @@ -365,6 +366,16 @@ json_object_object_add (json, "events", json_events); + json_job_environ = job_class_serialise_job_environ (); + + if (! json_job_environ) { + nih_error ("%s global job environment", + _("Failed to serialise")); + goto error; + } + + json_object_object_add (json, "job_environment", json_job_environ); + json_classes = job_class_serialise_all (); if (! json_classes) { @@ -415,6 +426,7 @@ { int ret = -1; json_object *json; + json_object *json_job_environ; enum json_tokener_error error; nih_assert (state); @@ -458,6 +470,18 @@ nih_warn ("%s", _("No ConfSources present in state data")); } + json_job_environ = json_object_object_get (json, "job_environment"); + + if (json_job_environ) { + if (job_class_deserialise_job_environ (json_job_environ) < 0) { + nih_error ("%s global job environment", + _("Failed to deserialise")); + goto out; + } + } else { + nih_warn ("%s", _("No global job environment data present in state data")); + } + if (job_class_deserialise_all (json) < 0) { nih_error ("%s JobClasses", _("Failed to deserialise")); goto out; @@ -610,7 +634,7 @@ nih_assert (len); if (! state_check_json_type (json, array)) - goto error; + return -1; *len = json_object_array_length (json); === modified file 'test/test_util_common.c' --- test/test_util_common.c 2013-10-02 08:59:20 +0000 +++ test/test_util_common.c 2013-10-11 16:20:01 +0000 @@ -119,6 +119,60 @@ TEST_EQ (running, TRUE); } +/** + * session_init_reexec: + * + * @pid: pid of Session Init. + * + * Cause the Session Init running as pid @pid to re-exec. + **/ +void +session_init_reexec (pid_t pid) +{ + nih_local NihDBusProxy *upstart = NULL; + char *address; + DBusConnection *connection; + DBusMessage *method_call; + DBusMessageIter iter; + DBusError error; + + TEST_TRUE (pid); + + TEST_TRUE (set_upstart_session (pid)); + address = getenv ("UPSTART_SESSION"); + + TEST_TRUE (address); + + connection = nih_dbus_connect (address, NULL); + TEST_NE_P (connection, NULL); + + upstart = nih_dbus_proxy_new (NULL, connection, + NULL, + DBUS_PATH_UPSTART, + NULL, NULL); + TEST_NE_P (upstart, NULL); + + method_call = dbus_message_new_method_call (upstart->name, + upstart->path, + "com.ubuntu.Upstart0_6", "Restart"); + TEST_NE_P (method_call, NULL); + + dbus_message_set_auto_start (method_call, upstart->auto_start); + + dbus_message_iter_init_append (method_call, &iter); + + /* Send the message, and wait for the reply. */ + dbus_error_init (&error); + + /* Don't bother checking reply - Upstart will sever the + * connection anyway. + */ + (void)dbus_connection_send_with_reply_and_block (upstart->connection, method_call, -1, &error); + + dbus_message_unref (method_call); + dbus_connection_unref (connection); +} + /* TRUE to denote that Upstart is running in user session mode * (FALSE to denote it's using the users D-Bus session bus). */ @@ -162,44 +216,44 @@ * within a reasonable period of time. */ for (i = 0; i < loops; i++) { - sleep (1); + sleep (1); RUN_COMMAND (NULL, cmd, &output, &lines); - if (lines < 1) - continue; - - /* Look for the specific session */ - for (size_t line = 0; line < lines; lines++) { - - /* No pid in output */ - if (! isdigit(output[line][0])) - continue; - - pid = (pid_t)atoi(output[line]); - nih_assert (pid > 0); - - if (pid != session_init_pid) - continue; - - /* look for separator between pid and value of - * UPSTART_SESSION. - */ - value = strstr (output[0], " "); - if (! value) - continue; - - /* jump over space */ - value += 1; - if (! value) - continue; - - /* No socket address */ - if (strstr (value, "unix:abstract") == value) { - got = TRUE; - goto out; - } - } + if (lines < 1) + continue; + + /* Look for the specific session */ + for (size_t line = 0; line < lines; lines++) { + + /* No pid in output */ + if (! isdigit(output[line][0])) + continue; + + pid = (pid_t)atoi(output[line]); + nih_assert (pid > 0); + + if (pid != session_init_pid) + continue; + + /* look for separator between pid and value of + * UPSTART_SESSION. + */ + value = strstr (output[0], " "); + if (! value) + continue; + + /* jump over space */ + value += 1; + if (! value) + continue; + + /* No socket address */ + if (strstr (value, "unix:abstract") == value) { + got = TRUE; + goto out; + } + } } out: === modified file 'test/test_util_common.h' --- test/test_util_common.h 2013-09-26 16:33:07 +0000 +++ test/test_util_common.h 2013-10-11 16:20:01 +0000 @@ -389,7 +389,11 @@ * Force upstart to perform a re-exec. **/ #define REEXEC_UPSTART(pid, user) \ - KILL_UPSTART (pid, SIGTERM, FALSE); \ + if (user) { \ + session_init_reexec (pid); \ + } else { \ + KILL_UPSTART (pid, SIGTERM, FALSE); \ + } \ wait_for_upstart (user ? pid : FALSE) /** @@ -691,6 +695,7 @@ __attribute__ ((warn_unused_result)); void wait_for_upstart (int session_init_pid); +void session_init_reexec (int session_init_pid); pid_t timed_waitpid (pid_t pid, time_t timeout) __attribute__ ((warn_unused_result)); === modified file 'util/man/initctl.8' --- util/man/initctl.8 2013-05-31 15:41:20 +0000 +++ util/man/initctl.8 2013-10-11 16:20:01 +0000 @@ -606,7 +606,8 @@ .RI [ OPTIONS "] " VARIABLE[=VALUE] Adds or updates a variable in a job environment table. Variables set -in this way will apply to all subsequently-starting jobs. +in this way will apply to all the subsequently-starting processes for a +job. This command is only permitted When running in .B User Session Mode. === modified file 'util/tests/test_initctl.c' --- util/tests/test_initctl.c 2013-09-26 16:33:07 +0000 +++ util/tests/test_initctl.c 2013-10-11 16:20:01 +0000 @@ -10987,6 +10987,109 @@ TEST_EQ (unsetenv ("UPSTART_CONFDIR"), 0); TEST_EQ (unsetenv ("UPSTART_LOGDIR"), 0); + + /*******************************************************************/ + TEST_FEATURE ("ensure 'set-env' persists across session-init re-exec"); + + contents = nih_sprintf (NULL, + "start on startup\n" + "\n" + "pre-start script\n" + "initctl set-env foo=bar\n" + "\n" + "# create flag file\n" + "touch \"%s\"\n" + "\n" + "end script\n" + "\n" + "# a minimal main process\n" + "exec true\n" + "\n" + "post-stop script\n" + "\n" + "# wait for upstart to re-exec before moving\n" + "# to next job process\n" + "while [ -f \"%s\" ]\n" + "do\n" + " sleep 0.1\n" + "done\n" + "\n" + "end script\n" + "\n" + "script\n" + "\n" + "# query value post-re-exec\n" + "initctl get-env foo\n" + "\n" + "end script\n", + flagfile, flagfile); + TEST_NE_P (contents, NULL); + + CREATE_FILE (confdir, "foo.conf", contents); + + start_upstart_common (&upstart_pid, TRUE, confdir, logdir, NULL); + + WAIT_FOR_FILE (flagfile); + + /* check job is running */ + job_pid = job_to_pid ("foo"); + TEST_NE (job_pid, -1); + + REEXEC_UPSTART (upstart_pid, TRUE); + + /* Notify job that upstart has re-exec'd to allow it to move + * out of pre-start. + */ + assert0 (unlink (flagfile)); + + logfile = NIH_MUST (nih_sprintf (NULL, "%s/%s", + logdir, + "foo.log")); + + WAIT_FOR_FILE (logfile); + file = fopen (logfile, "r"); + TEST_NE_P (file, NULL); + TEST_FILE_EQ (file, "bar\r\n"); + fclose (file); + + STOP_UPSTART (upstart_pid); + assert0 (unlink (logfile)); + DELETE_FILE (confdir, "foo.conf"); + + /*******************************************************************/ + TEST_FEATURE ("ensure 'set-env --global' persists across session-init re-exec"); + + START_UPSTART (upstart_pid, TRUE); + + /* Set variable. Use confdir as a random value */ + cmd = nih_sprintf (NULL, "%s set-env --global path='%s' 2>&1", get_initctl (), confdir); + TEST_NE_P (cmd, NULL); + RUN_COMMAND (NULL, cmd, &output, &lines); + TEST_EQ (lines, 0); + + /* Check it */ + cmd = nih_sprintf (NULL, "%s get-env --global path 2>&1", get_initctl ()); + TEST_NE_P (cmd, NULL); + RUN_COMMAND (NULL, cmd, &output, &lines); + TEST_EQ (lines, 1); + TEST_STR_MATCH (output[0], confdir); + nih_free (output); + + /* Restart */ + REEXEC_UPSTART (upstart_pid, TRUE); + + /* Re-check */ + cmd = nih_sprintf (NULL, "%s get-env --global path 2>&1", get_initctl ()); + TEST_NE_P (cmd, NULL); + RUN_COMMAND (NULL, cmd, &output, &lines); + TEST_EQ (lines, 1); + TEST_STR_MATCH (output[0], confdir); + nih_free (output); + + STOP_UPSTART (upstart_pid); + + /*******************************************************************/ + TEST_DBUS_END (dbus_pid); TEST_EQ (rmdir (confdir), 0); @@ -16503,7 +16606,6 @@ assert0 (unlink (logfile)); DELETE_FILE (confdir, "bar.conf"); - /*******************************************************************/ } @@ -16663,6 +16765,8 @@ test_global_and_local_job_env (confdir, logdir, upstart_pid, dbus_pid); + //test_reexec_job_env (confdir, logdir, upstart_pid, dbus_pid); + /*******************************************************************/ STOP_UPSTART (upstart_pid);
-- upstart-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/upstart-devel
