Frequently, when the NTP daemon is run from connman in one-shot mode
(aka ntpdate mode) to synchronize the time against one or more peers,
NTP can be timed out in its ntp_intres handler, attempting to resolve
a peer by name (e.g. ntp.ubuntu.com), leaving two NTP processes
running:
154 ? S 0:00 /usr/sbin/ntpd -g -q -c /tmp/connman.ntp.conf_6W
158 ? S 0:00 /usr/sbin/ntpd -g -q -c /tmp/connman.ntp.conf_6W
In this state, the next time connman starts, any attempts to
synchronize the time will fail due to the above pended task.
Presently, when connman quits, these pended processes are not cleaned
up. With this patch, the NTP plugin attempts to stop and then destroy
the previously-started NTP task. The secondary ntp_intres process will
notice that its parent has quit and will quit in reaction.
---
plugins/ntpd.c | 88 +++++++++++++++++++++++++++++++++++++-------------------
1 files changed, 58 insertions(+), 30 deletions(-)
diff --git a/plugins/ntpd.c b/plugins/ntpd.c
index 3ee2ad8..358fe7e 100644
--- a/plugins/ntpd.c
+++ b/plugins/ntpd.c
@@ -37,13 +37,6 @@
#include <connman/driver.h>
#include <connman/log.h>
-/*
- * The peers list are the peers currently added to a running ntpd,
- * while pending_peers are the one appended but not used by ntpd yet.
- */
-static GList *peers = NULL;
-static GList *pending_peers = NULL;
-
#define NTPD_PORT 123
struct ntpd_peer {
@@ -57,6 +50,14 @@ struct ntpdate_task {
char *conf_path;
};
+/*
+ * The peers list are the peers currently added to a running ntpd,
+ * while pending_peers are the one appended but not used by ntpd yet.
+ */
+static GList *peers = NULL;
+static GList *pending_peers = NULL;
+static struct ntpdate_task *ntpdtask = NULL;
+
static struct ntpd_peer *find_peer(GList *peer_list, const char* server)
{
GList *list;
@@ -112,13 +113,21 @@ static connman_bool_t ntpd_running(void)
static void ntpdate_died(struct connman_task *task,
int exit_code, void *user_data)
{
- struct ntpdate_task *ntpdate = user_data;
+ struct ntpdate_task *ntpdate = *(struct ntpdate_task **)user_data;
- DBG("");
+ DBG("exit_code %d user_data %p", exit_code, user_data);
- unlink(ntpdate->conf_path);
- g_free(ntpdate->conf_path);
- connman_task_destroy(ntpdate->task);
+ if (ntpdate != NULL) {
+ unlink(ntpdate->conf_path);
+ g_free(ntpdate->conf_path);
+ ntpdate->conf_path = NULL;
+
+ connman_task_destroy(ntpdate->task);
+ ntpdate->task = NULL;
+
+ g_free(ntpdate);
+ ntpdate = NULL;
+ }
}
static void ntpdate_add_peer(struct ntpdate_task *ntpdate, char *peer)
@@ -144,58 +153,59 @@ static int ntpdate(void)
GError *g_err;
GList *list;
struct ntpd_peer *peer;
- struct ntpdate_task *ntpdate;
DBG("");
- ntpdate = g_try_new0(struct ntpdate_task, 1);
- if (ntpdate == NULL)
+ ntpdtask = g_try_new0(struct ntpdate_task, 1);
+ if (ntpdtask == NULL)
return -ENOMEM;
/* ntpdate is deprecated, we use ntpd -q instead */
- ntpdate->task = connman_task_create(NTPD);
- if (ntpdate->task == NULL) {
+ ntpdtask->task = connman_task_create(NTPD);
+ if (ntpdtask->task == NULL) {
err = -ENOMEM;
goto error_task;
}
- connman_task_add_argument(ntpdate->task, "-g", NULL);
- connman_task_add_argument(ntpdate->task, "-q", NULL);
+ connman_task_add_argument(ntpdtask->task, "-g", NULL);
+ connman_task_add_argument(ntpdtask->task, "-q", NULL);
/* The servers are added through a temp configuration file */
- ntpdate->conf_fd = g_file_open_tmp("connman.ntp.conf_XXXXXX",
- &ntpdate->conf_path, &g_err);
- if (ntpdate->conf_fd == -1) {
+ ntpdtask->conf_fd = g_file_open_tmp("connman.ntp.conf_XXXXXX",
+ &ntpdtask->conf_path, &g_err);
+ if (ntpdtask->conf_fd == -1) {
err = g_err->code;
g_free(g_err);
goto error_open;
}
- connman_task_add_argument(ntpdate->task, "-c", ntpdate->conf_path);
+ connman_task_add_argument(ntpdtask->task, "-c", ntpdtask->conf_path);
- DBG("conf path %s", ntpdate->conf_path);
+ DBG("conf path %s", ntpdtask->conf_path);
for (list = pending_peers; list; list = list->next) {
peer = list->data;
- ntpdate_add_peer(ntpdate, peer->server);
+ ntpdate_add_peer(ntpdtask, peer->server);
}
for (list = peers; list; list = list->next) {
peer = list->data;
- ntpdate_add_peer(ntpdate, peer->server);
+ ntpdate_add_peer(ntpdtask, peer->server);
}
- close(ntpdate->conf_fd);
+ close(ntpdtask->conf_fd);
- return connman_task_run(ntpdate->task, ntpdate_died, ntpdate,
+ return connman_task_run(ntpdtask->task, ntpdate_died, &ntpdtask,
NULL, NULL, NULL);
error_open:
- connman_task_destroy(ntpdate->task);
+ connman_task_destroy(ntpdtask->task);
+ ntpdtask->task = NULL;
error_task:
- g_free(ntpdate);
+ g_free(ntpdtask);
+ ntpdtask = NULL;
return err;
}
@@ -315,6 +325,24 @@ static int ntpd_init(void)
static void ntpd_exit(void)
{
+ DBG("ntpdtask %p", ntpdtask);
+
+ if (ntpdtask != NULL) {
+ connman_task_stop(ntpdtask->task);
+
+ if (ntpdtask->conf_path != NULL) {
+ unlink(ntpdtask->conf_path);
+ g_free(ntpdtask->conf_path);
+ ntpdtask->conf_path = NULL;
+ }
+
+ connman_task_destroy(ntpdtask->task);
+ ntpdtask->task = NULL;
+
+ g_free(ntpdtask);
+ ntpdtask = NULL;
+ }
+
connman_timeserver_driver_unregister(&ntpd_driver);
}
--
1.7.4.2
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman
