daemon: Don't hold settings lock while executing start/stop scripts

If a called script interacts with the daemon or one of its plugins
another thread might have to acquire the write lock (e.g. to configure a
fallback or set a value).  Holding the read lock prevents that, potentially
resulting in a deadlock.
This commit is contained in:
Tobias Brunner
2016-06-17 18:43:35 +02:00
parent 44e83f76f3
commit 941ac92b95
+43 -20
View File
@@ -1,9 +1,9 @@
/* /*
* Copyright (C) 2006-2015 Tobias Brunner * Copyright (C) 2006-2016 Tobias Brunner
* Copyright (C) 2005-2009 Martin Willi * Copyright (C) 2005-2009 Martin Willi
* Copyright (C) 2006 Daniel Roethlisberger * Copyright (C) 2006 Daniel Roethlisberger
* Copyright (C) 2005 Jan Hutter * Copyright (C) 2005 Jan Hutter
* Hochschule fuer Technik Rapperswil * HSR Hochschule fuer Technik Rapperswil
* *
* This program is free software; you can redistribute it and/or modify it * This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the * under the terms of the GNU General Public License as published by the
@@ -54,6 +54,7 @@
#include <library.h> #include <library.h>
#include <bus/listeners/sys_logger.h> #include <bus/listeners/sys_logger.h>
#include <bus/listeners/file_logger.h> #include <bus/listeners/file_logger.h>
#include <collections/array.h>
#include <config/proposal.h> #include <config/proposal.h>
#include <plugins/plugin_feature.h> #include <plugins/plugin_feature.h>
#include <kernel/kernel_handler.h> #include <kernel/kernel_handler.h>
@@ -701,46 +702,68 @@ static void destroy(private_daemon_t *this)
*/ */
static void run_scripts(private_daemon_t *this, char *verb) static void run_scripts(private_daemon_t *this, char *verb)
{ {
struct {
char *name;
char *path;
} *script;
array_t *scripts = NULL;
enumerator_t *enumerator; enumerator_t *enumerator;
char *key, *value, *pos, buf[1024]; char *key, *value, *pos, buf[1024];
FILE *cmd; FILE *cmd;
/* copy the scripts so we don't hold any locks while executing them */
enumerator = lib->settings->create_key_value_enumerator(lib->settings, enumerator = lib->settings->create_key_value_enumerator(lib->settings,
"%s.%s-scripts", lib->ns, verb); "%s.%s-scripts", lib->ns, verb);
while (enumerator->enumerate(enumerator, &key, &value)) while (enumerator->enumerate(enumerator, &key, &value))
{ {
DBG1(DBG_DMN, "executing %s script '%s' (%s):", verb, key, value); INIT(script,
cmd = popen(value, "r"); .name = key,
.path = value,
);
array_insert_create(&scripts, ARRAY_TAIL, script);
}
enumerator->destroy(enumerator);
enumerator = array_create_enumerator(scripts);
while (enumerator->enumerate(enumerator, &script))
{
DBG1(DBG_DMN, "executing %s script '%s' (%s)", verb, script->name,
script->path);
cmd = popen(script->path, "r");
if (!cmd) if (!cmd)
{ {
DBG1(DBG_DMN, "executing %s script '%s' (%s) failed: %s", DBG1(DBG_DMN, "executing %s script '%s' (%s) failed: %s",
verb, key, value, strerror(errno)); verb, script->name, script->path, strerror(errno));
continue;
} }
while (TRUE) else
{ {
if (!fgets(buf, sizeof(buf), cmd)) while (TRUE)
{ {
if (ferror(cmd)) if (!fgets(buf, sizeof(buf), cmd))
{ {
DBG1(DBG_DMN, "reading from %s script '%s' (%s) failed", if (ferror(cmd))
verb, key, value); {
DBG1(DBG_DMN, "reading from %s script '%s' (%s) failed",
verb, script->name, script->path);
}
break;
} }
break; else
}
else
{
pos = buf + strlen(buf);
if (pos > buf && pos[-1] == '\n')
{ {
pos[-1] = '\0'; pos = buf + strlen(buf);
if (pos > buf && pos[-1] == '\n')
{
pos[-1] = '\0';
}
DBG1(DBG_DMN, "%s: %s", script->name, buf);
} }
DBG1(DBG_DMN, "%s: %s", key, buf);
} }
pclose(cmd);
} }
pclose(cmd); free(script);
} }
enumerator->destroy(enumerator); enumerator->destroy(enumerator);
array_destroy(scripts);
} }
METHOD(daemon_t, start, void, METHOD(daemon_t, start, void,