Introduce sd_notify_barrier

This adds the sd_notify_barrier function, to allow users to synchronize against
the reception of sd_notify(3) status messages. It acts as a synchronization
point, and a successful return gurantees that all previous messages have been
consumed by the manager. This can be used to eliminate race conditions where
the sending process exits too early for systemd to associate its PID to a
cgroup and attribute the status message to a unit correctly.

systemd-notify now uses this function for proper notification delivery and be
useful for NotifyAccess=all units again in user mode, or in cases where it
doesn't have a control process as parent.

Fixes: #2739
This commit is contained in:
Kumar Kartikeya Dwivedi 2020-04-28 19:39:27 +05:30
parent cad6727906
commit 4f07ddfa9b
No known key found for this signature in database
GPG Key ID: 4CE0C88648BF11CA
6 changed files with 130 additions and 1 deletions

View File

@ -718,7 +718,7 @@ manpages = [
['sd_machine_get_class', '3', ['sd_machine_get_ifindices'], ''],
['sd_notify',
'3',
['sd_notifyf', 'sd_pid_notify', 'sd_pid_notify_with_fds', 'sd_pid_notifyf'],
['sd_notifyf', 'sd_pid_notify', 'sd_pid_notify_with_fds', 'sd_pid_notifyf', 'sd_notify_barrier'],
''],
['sd_path_lookup', '3', ['sd_path_lookup_strv'], ''],
['sd_pid_get_owner_uid',

View File

@ -22,6 +22,7 @@
<refname>sd_pid_notify</refname>
<refname>sd_pid_notifyf</refname>
<refname>sd_pid_notify_with_fds</refname>
<refname>sd_notify_barrier</refname>
<refpurpose>Notify service manager about start-up completion and other service status changes</refpurpose>
</refnamediv>
@ -65,6 +66,12 @@
<paramdef>const int *<parameter>fds</parameter></paramdef>
<paramdef>unsigned <parameter>n_fds</parameter></paramdef>
</funcprototype>
<funcprototype>
<funcdef>int <function>sd_notify_barrier</function></funcdef>
<paramdef>int <parameter>unset_environment</parameter></paramdef>
<paramdef>uint64_t <parameter>timeout</parameter></paramdef>
</funcprototype>
</funcsynopsis>
</refsynopsisdiv>
@ -261,6 +268,17 @@
as prematurely discarding file descriptors from the store.</para></listitem>
</varlistentry>
<varlistentry>
<term>BARRIER=1</term>
<listitem><para>Tells the service manager that the client is explicitly requesting synchronization by means of
closing the file descriptor sent with this command. The service manager gurantees that the processing of a <varname>
BARRIER=1</varname> command will only happen after all previous notification messages sent before this command
have been processed. Hence, this command accompanied with a single file descriptor can be used to synchronize
against reception of all previous status messages. Note that this command cannot be mixed with other notifications,
and has to be sent in a separate message to the service manager, otherwise all assignments will be ignored. Note that
sending 0 or more than 1 file descriptor with this command is a violation of the protocol.</para></listitem>
</varlistentry>
</variablelist>
<para>It is recommended to prefix variable names that are not
@ -282,6 +300,13 @@
attribute the message to the unit, and thus will ignore it, even if
<varname>NotifyAccess=</varname><option>all</option> is set for it.</para>
<para>Hence, to eliminate all race conditions involving lookup of the client's unit and attribution of notifications
to units correctly, <function>sd_notify_barrier()</function> may be used. This call acts as a synchronization point
and ensures all notifications sent before this call have been picked up by the service manager when it returns
successfully. Use of <function>sd_notify_barrier()</function> is needed for clients which are not invoked by the
service manager, otherwise this synchronization mechanism is unnecessary for attribution of notifications to the
unit.</para>
<para><function>sd_notifyf()</function> is similar to
<function>sd_notify()</function> but takes a
<function>printf()</function>-like format string plus
@ -312,6 +337,14 @@
to the service manager on messages that do not expect them (i.e.
without <literal>FDSTORE=1</literal>) they are immediately closed
on reception.</para>
<para><function>sd_notify_barrier()</function> allows the caller to
synchronize against reception of previously sent notification messages
and uses the <literal>BARRIER=1</literal> command. It takes a relative
<varname>timeout</varname> value in microseconds which is passed to
<citerefentry><refentrytitle>ppoll</refentrytitle><manvolnum>2</manvolnum>
</citerefentry>. A value of UINT64_MAX is interpreted as infinite timeout.
</para>
</refsect1>
<refsect1>
@ -402,6 +435,22 @@
<programlisting>sd_pid_notify_with_fds(0, 0, "FDSTORE=1\nFDNAME=foobar", &amp;fd, 1);</programlisting>
</example>
<example>
<title>Eliminating race conditions</title>
<para>When the client sending the notifications is not spawned
by the service manager, it may exit too quickly and the service
manager may fail to attribute them correctly to the unit. To
prevent such races, use <function>sd_notify_barrier()</function>
to synchronize against reception of all notifications sent before
this call is made.</para>
<programlisting>sd_notify(0, "READY=1");
/* set timeout to 5 seconds */
sd_notify_barrier(0, 5 * 1000000);
</programlisting>
</example>
</refsect1>
<refsect1>

View File

@ -2284,6 +2284,20 @@ static int manager_dispatch_cgroups_agent_fd(sd_event_source *source, int fd, ui
return 0;
}
static bool manager_process_barrier_fd(const char *buf, FDSet *fds) {
assert(buf);
/* nothing else must be sent when using BARRIER=1 */
if (STR_IN_SET(buf, "BARRIER=1", "BARRIER=1\n")) {
if (fdset_size(fds) != 1)
log_warning("Got incorrect number of fds with BARRIER=1, closing them.");
return true;
} else if (startswith(buf, "BARRIER=1\n") || strstr(buf, "\nBARRIER=1\n") || endswith(buf, "\nBARRIER=1"))
log_warning("Extra notification messages sent with BARRIER=1, ignoring everything.");
return false;
}
static void manager_invoke_notify_message(
Manager *m,
Unit *u,
@ -2417,6 +2431,10 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t
/* Make sure it's NUL-terminated. */
buf[n] = 0;
/* possibly a barrier fd, let's see */
if (manager_process_barrier_fd(buf, fds))
return 0;
/* Increase the generation counter used for filtering out duplicate unit invocations. */
m->notifygen++;

View File

@ -4,6 +4,7 @@
#include <limits.h>
#include <mqueue.h>
#include <netinet/in.h>
#include <poll.h>
#include <stdarg.h>
#include <stddef.h>
#include <stdio.h>
@ -23,6 +24,7 @@
#include "process-util.h"
#include "socket-util.h"
#include "strv.h"
#include "time-util.h"
#include "util.h"
#define SNDBUF_SIZE (8*1024*1024)
@ -551,6 +553,34 @@ finish:
return r;
}
_public_ int sd_notify_barrier(int unset_environment, uint64_t timeout) {
_cleanup_close_pair_ int pipe_fd[2] = { -1, -1 };
struct timespec ts;
int r;
if (pipe2(pipe_fd, O_CLOEXEC) < 0)
return -errno;
r = sd_pid_notify_with_fds(0, unset_environment, "BARRIER=1", &pipe_fd[1], 1);
if (r <= 0)
return r;
pipe_fd[1] = safe_close(pipe_fd[1]);
struct pollfd pfd = {
.fd = pipe_fd[0],
/* POLLHUP is implicit */
.events = 0,
};
r = ppoll(&pfd, 1, timeout == UINT64_MAX ? NULL : timespec_store(&ts, timeout), NULL);
if (r < 0)
return -errno;
if (r == 0)
return -ETIMEDOUT;
return 1;
}
_public_ int sd_pid_notify(pid_t pid, int unset_environment, const char *state) {
return sd_pid_notify_with_fds(pid, unset_environment, state, NULL, 0);
}

View File

@ -18,6 +18,7 @@
#include "string-util.h"
#include "strv.h"
#include "terminal-util.h"
#include "time-util.h"
#include "user-util.h"
#include "util.h"
@ -27,6 +28,7 @@ static const char *arg_status = NULL;
static bool arg_booted = false;
static uid_t arg_uid = UID_INVALID;
static gid_t arg_gid = GID_INVALID;
static bool arg_no_block = false;
static int help(void) {
_cleanup_free_ char *link = NULL;
@ -45,6 +47,7 @@ static int help(void) {
" --uid=USER Set user to send from\n"
" --status=TEXT Set status text\n"
" --booted Check if the system was booted up with systemd\n"
" --no-block Do not wait until operation finished\n"
"\nSee the %s for details.\n"
, program_invocation_short_name
, ansi_highlight(), ansi_normal()
@ -83,6 +86,7 @@ static int parse_argv(int argc, char *argv[]) {
ARG_STATUS,
ARG_BOOTED,
ARG_UID,
ARG_NO_BLOCK
};
static const struct option options[] = {
@ -93,6 +97,7 @@ static int parse_argv(int argc, char *argv[]) {
{ "status", required_argument, NULL, ARG_STATUS },
{ "booted", no_argument, NULL, ARG_BOOTED },
{ "uid", required_argument, NULL, ARG_UID },
{ "no-block", no_argument, NULL, ARG_NO_BLOCK },
{}
};
@ -157,6 +162,10 @@ static int parse_argv(int argc, char *argv[]) {
break;
}
case ARG_NO_BLOCK:
arg_no_block = true;
break;
case '?':
return -EINVAL;
@ -256,6 +265,16 @@ static int run(int argc, char* argv[]) {
if (r == 0)
return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP),
"No status data could be sent: $NOTIFY_SOCKET was not set");
if (!arg_no_block) {
r = sd_notify_barrier(0, 5 * USEC_PER_SEC);
if (r < 0)
return log_error_errno(r, "Failed to invoke barrier: %m");
if (r == 0)
return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP),
"No status data could be sent: $NOTIFY_SOCKET was not set");
}
return 0;
}

View File

@ -286,6 +286,19 @@ int sd_pid_notifyf(pid_t pid, int unset_environment, const char *format, ...) _s
*/
int sd_pid_notify_with_fds(pid_t pid, int unset_environment, const char *state, const int *fds, unsigned n_fds);
/*
Returns > 0 if synchronization with systemd succeeded. Returns < 0
on error. Returns 0 if $NOTIFY_SOCKET was not set. Note that the
timeout parameter of this function call takes the timeout in µs, and
will be passed to ppoll(2), hence the behaviour will be similar to
ppoll(2). This function can be called after sending a status message
to systemd, if one needs to synchronize against reception of the
status messages sent before this call is made. Therefore, this
cannot be used to know if the status message was processed
successfully, but to only synchronize against its consumption.
*/
int sd_notify_barrier(int unset_environment, uint64_t timeout);
/*
Returns > 0 if the system was booted with systemd. Returns < 0 on
error. Returns 0 if the system was not booted with systemd. Note