Merge lp:~jamesodhunt/upstart/restrict-debug-stanza into lp:upstart

Proposed by James Hunt on 2014-01-31
Status: Merged
Merged at revision: 1605
Proposed branch: lp:~jamesodhunt/upstart/restrict-debug-stanza
Merge into: lp:upstart
Diff against target: 82 lines (+26/-2)
4 files modified
ChangeLog (+8/-0)
init/job_process.c (+6/-0)
init/main.c (+4/-0)
init/parse_job.c (+8/-2)
To merge this branch: bzr merge lp:~jamesodhunt/upstart/restrict-debug-stanza
Reviewer Review Type Date Requested Status
Upstart Reviewers 2014-01-31 Pending
Review via email: mp+204363@code.launchpad.net

Description of the change

Creating a job with a debug stanza will stop stateful re-exec from working reliably since the child is paused before exec'ing such that it still retains a copy of the parents file descriptors. This is legitimate since that is the whole point of the unofficial debug stanza. Since the debug stanza is only to be used for development, no production job should be using it and in fact no production version of Upstart that supports stateful re-exec can be using it due to the problems it causes with re-exec.

As such, this branch makes the debug stanza a NOP unless upstart was *started* with --debug.

To post a comment you must log in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ChangeLog'
2--- ChangeLog 2014-01-22 11:16:20 +0000
3+++ ChangeLog 2014-01-31 23:49:11 +0000
4@@ -1,3 +1,11 @@
5+2014-01-31 James Hunt <james.hunt@ubuntu.com>
6+
7+ * init/job_process.c: job_process_spawn(): Comments.
8+ * init/main.c: main(): Enable the debug stanza when booting with
9+ '--debug' (switching log-level will not work).
10+ * init/parse_job.c: stanza_debug(): Conditionally disable the debug
11+ stanza.
12+
13 2014-01-22 James Hunt <james.hunt@ubuntu.com>
14
15 * init/main.c: logger_kmsg(): Use open(2) rather than fopen(3) to avoid
16
17=== modified file 'init/job_process.c'
18--- init/job_process.c 2013-11-03 02:54:03 +0000
19+++ init/job_process.c 2014-01-31 23:49:11 +0000
20@@ -915,6 +915,12 @@
21 * fail. Note that closing the pipe means from this point onwards,
22 * the parent cannot know the true outcome of the spawn: that
23 * responsibility lies with the debugger.
24+ *
25+ * - note that running with the debug stanza enabled will
26+ * unavoidably stop stateful re-exec from working correctly
27+ * since the stopped debug child process will hold copies of
28+ * the parents file descriptors open until it continues to
29+ * call exec below.
30 */
31 if (class->debug) {
32 close (fds[1]);
33
34=== modified file 'init/main.c'
35--- init/main.c 2014-01-22 13:49:01 +0000
36+++ init/main.c 2014-01-31 23:49:11 +0000
37@@ -138,6 +138,7 @@
38 extern char *log_dir;
39 extern DBusBusType dbus_bus_type;
40 extern mode_t initial_umask;
41+extern int debug_stanza_enabled;
42
43 /**
44 * options:
45@@ -219,6 +220,9 @@
46 if (! args)
47 exit (1);
48
49+ if (nih_log_priority == NIH_LOG_DEBUG)
50+ debug_stanza_enabled = TRUE;
51+
52 handle_confdir ();
53 handle_logdir ();
54
55
56=== modified file 'init/parse_job.c'
57--- init/parse_job.c 2013-07-21 23:54:16 +0000
58+++ init/parse_job.c 2014-01-31 23:49:11 +0000
59@@ -240,7 +240,12 @@
60 size_t *pos, size_t *lineno)
61 __attribute__ ((warn_unused_result));
62
63-
64+/**
65+ * debug_stanza_enabled:
66+ *
67+ * If TRUE, honour the debug stanza, else consider it a NOP.
68+ **/
69+int debug_stanza_enabled = FALSE;
70
71 /**
72 * stanzas:
73@@ -1084,7 +1089,8 @@
74 nih_assert (file != NULL);
75 nih_assert (pos != NULL);
76
77- class->debug = TRUE;
78+ if (debug_stanza_enabled)
79+ class->debug = TRUE;
80
81 return 0;
82 }

Subscribers

People subscribed via source and target branches