[systemd-devel] [PATCHv4] core: notify triggered by socket of a service
Colin Guthrie
gmane at colin.guthr.ie
Wed Sep 18 16:54:36 PDT 2013
Hi,
While I'm not 100% sure, (not done quite enough testing), but this patch
seems to have introduced some kind of race that results in me seeing a
1min delay in boot. This delay creeps in before
systemd-udevd-control.socket starts so my dmesg looks like:
[ 1.549666] RPC: Registered tcp NFSv4.1 backchannel transport module.
[ 61.592613] systemd[1]: ConditionPathIsReadWrite=/sys succeeded
for systemd-udevd-control.socket.
And it was the mention socket related stuff that reminded me of this
thread and patch.
So I reverted the commit (otherwise running git master) and have
performed three successful boots so far without issue. This is obviously
not conclusive, but sadly I also have a problem shutting down quickly...
seems my httpd.service file now also stalls until it times out. When the
ControlGroup becomes empty, it seems this isn't properly detected and
systemd keeps the unit in the "deactivating" state until it times out.
I'll post a separate mail about this issue.
I'll keep testing and reporting if I still get the delayed boot.
Col
'Twas brillig, and Lennart Poettering at 12/09/13 17:47 did gyre and gimble:
> On Mon, 22.07.13 10:52, Umut Tezduyar (umut at tezduyar.com) wrote:
>
> Thanks!
>
> Applied!
>
>> ---
>> TODO | 3 ---
>> src/core/service.c | 31 -------------------------------
>> src/core/socket.c | 39 ++++++++++++++++++++++++++++++++++++++-
>> src/core/socket.h | 3 ---
>> 4 files changed, 38 insertions(+), 38 deletions(-)
>>
>> diff --git a/TODO b/TODO
>> index ba8bb8e..e0c4857 100644
>> --- a/TODO
>> +++ b/TODO
>> @@ -115,9 +115,6 @@ Features:
>> Maybe take a BSD lock at the disk device node and teach udev to
>> check for that and suppress event handling.
>>
>> -* when a service changes state make reflect that in the
>> - RUNNING/LISTENING states of its socket
>> -
>> * when recursively showing the cgroup hierarchy, optionally also show
>> the hierarchies of child processes
>>
>> diff --git a/src/core/service.c b/src/core/service.c
>> index b98f11a..157d614 100644
>> --- a/src/core/service.c
>> +++ b/src/core/service.c
>> @@ -1481,24 +1481,6 @@ static int service_search_main_pid(Service *s) {
>> return 0;
>> }
>>
>> -static void service_notify_sockets_dead(Service *s, bool failed_permanent) {
>> - Iterator i;
>> - Unit *u;
>> -
>> - assert(s);
>> -
>> - /* Notifies all our sockets when we die */
>> -
>> - if (s->socket_fd >= 0)
>> - return;
>> -
>> - SET_FOREACH(u, UNIT(s)->dependencies[UNIT_TRIGGERED_BY], i)
>> - if (u->type == UNIT_SOCKET)
>> - socket_notify_service_dead(SOCKET(u), failed_permanent);
>> -
>> - return;
>> -}
>> -
>> static void service_set_state(Service *s, ServiceState state) {
>> ServiceState old_state;
>> const UnitActiveState *table;
>> @@ -1550,19 +1532,6 @@ static void service_set_state(Service *s, ServiceState state) {
>> s->control_command_id = _SERVICE_EXEC_COMMAND_INVALID;
>> }
>>
>> - if (state == SERVICE_FAILED)
>> - service_notify_sockets_dead(s, s->result == SERVICE_FAILURE_START_LIMIT);
>> -
>> - if (state == SERVICE_DEAD ||
>> - state == SERVICE_STOP ||
>> - state == SERVICE_STOP_SIGTERM ||
>> - state == SERVICE_STOP_SIGKILL ||
>> - state == SERVICE_STOP_POST ||
>> - state == SERVICE_FINAL_SIGTERM ||
>> - state == SERVICE_FINAL_SIGKILL ||
>> - state == SERVICE_AUTO_RESTART)
>> - service_notify_sockets_dead(s, false);
>> -
>> if (state != SERVICE_START_PRE &&
>> state != SERVICE_START &&
>> state != SERVICE_START_POST &&
>> diff --git a/src/core/socket.c b/src/core/socket.c
>> index cf88bae..2130e48 100644
>> --- a/src/core/socket.c
>> +++ b/src/core/socket.c
>> @@ -2277,7 +2277,7 @@ int socket_collect_fds(Socket *s, int **fds, unsigned *n_fds) {
>> return 0;
>> }
>>
>> -void socket_notify_service_dead(Socket *s, bool failed_permanent) {
>> +static void socket_notify_service_dead(Socket *s, bool failed_permanent) {
>> assert(s);
>>
>> /* The service is dead. Dang!
>> @@ -2322,6 +2322,41 @@ static void socket_reset_failed(Unit *u) {
>> s->result = SOCKET_SUCCESS;
>> }
>>
>> +static void socket_trigger_notify(Unit *u, Unit *other) {
>> + Socket *s = SOCKET(u);
>> + Service *se = SERVICE(other);
>> +
>> + assert(u);
>> + assert(other);
>> +
>> + /* Don't propagate state changes from the service if we are
>> + already down or accepting connections */
>> + if ((s->state != SOCKET_RUNNING &&
>> + s->state != SOCKET_LISTENING) ||
>> + s->accept)
>> + return;
>> +
>> + if (other->load_state != UNIT_LOADED ||
>> + other->type != UNIT_SERVICE)
>> + return;
>> +
>> + if (se->state == SERVICE_FAILED)
>> + socket_notify_service_dead(s, se->result == SERVICE_FAILURE_START_LIMIT);
>> +
>> + if (se->state == SERVICE_DEAD ||
>> + se->state == SERVICE_STOP ||
>> + se->state == SERVICE_STOP_SIGTERM ||
>> + se->state == SERVICE_STOP_SIGKILL ||
>> + se->state == SERVICE_STOP_POST ||
>> + se->state == SERVICE_FINAL_SIGTERM ||
>> + se->state == SERVICE_FINAL_SIGKILL ||
>> + se->state == SERVICE_AUTO_RESTART)
>> + socket_notify_service_dead(s, false);
>> +
>> + if (se->state == SERVICE_RUNNING)
>> + socket_set_state(s, SOCKET_RUNNING);
>> +}
>> +
>> static int socket_kill(Unit *u, KillWho who, int signo, DBusError *error) {
>> return unit_kill_common(u, who, signo, -1, SOCKET(u)->control_pid, error);
>> }
>> @@ -2402,6 +2437,8 @@ const UnitVTable socket_vtable = {
>> .sigchld_event = socket_sigchld_event,
>> .timer_event = socket_timer_event,
>>
>> + .trigger_notify = socket_trigger_notify,
>> +
>> .reset_failed = socket_reset_failed,
>>
>> .bus_interface = "org.freedesktop.systemd1.Socket",
>> diff --git a/src/core/socket.h b/src/core/socket.h
>> index 8f9dfdb..5733322 100644
>> --- a/src/core/socket.h
>> +++ b/src/core/socket.h
>> @@ -156,9 +156,6 @@ struct Socket {
>> /* Called from the service code when collecting fds */
>> int socket_collect_fds(Socket *s, int **fds, unsigned *n_fds);
>>
>> -/* Called from the service when it shut down */
>> -void socket_notify_service_dead(Socket *s, bool failed_permanent);
>> -
>> /* Called from the mount code figure out if a mount is a dependency of
>> * any of the sockets of this socket */
>> int socket_add_one_mount_link(Socket *s, Mount *m);
>
>
> Lennart
>
--
Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/
Day Job:
Tribalogic Limited http://www.tribalogic.net/
Open Source:
Mageia Contributor http://www.mageia.org/
PulseAudio Hacker http://www.pulseaudio.org/
Trac Hacker http://trac.edgewall.org/
More information about the systemd-devel
mailing list