[master] b0a7454 Close a couple of races.
Poul-Henning Kamp
phk at FreeBSD.org
Sun Jan 15 19:28:04 CET 2017
commit b0a7454f1771f4cefddb6b1340667bedf8df015c
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Sun Jan 15 18:26:16 2017 +0000
Close a couple of races.
diff --git a/bin/varnishtest/vtc_process.c b/bin/varnishtest/vtc_process.c
index ee4dc05..ac6d4a3 100644
--- a/bin/varnishtest/vtc_process.c
+++ b/bin/varnishtest/vtc_process.c
@@ -62,8 +62,9 @@ struct process {
int fd_from;
pid_t pid;
+ pthread_mutex_t mtx;
pthread_t tp;
- unsigned running;
+ unsigned hasthread;
int status;
};
@@ -95,6 +96,7 @@ process_new(const char *name)
ALLOC_OBJ(p, PROCESS_MAGIC);
AN(p);
REPLACE(p->name, name);
+ AZ(pthread_mutex_init(&p->mtx, NULL));
p->vl = vtc_logopen(name);
AN(p->vl);
@@ -129,6 +131,7 @@ process_delete(struct process *p)
{
CHECK_OBJ_NOTNULL(p, PROCESS_MAGIC);
+ AZ(pthread_mutex_destroy(&p->mtx));
vtc_logclose(p->vl);
free(p->name);
free(p->workdir);
@@ -174,14 +177,22 @@ process_thread(void *priv)
continue;
}
r = wait4(p->pid, &p->status, 0, &ru);
+
+ closefd(&p->fd_to);
+
+ AZ(pthread_mutex_lock(&p->mtx));
+
macro_undef(p->vl, p->name, "pid");
p->pid = -1;
- p->running = 0;
- vtc_log(p->vl, 2, "R %d Status: %04x (u %.6f s %.6f)", r, p->status,
+
+ vtc_log(p->vl, 2, "R 0x%04x Status: %04x (u %.6f s %.6f)",
+ r, p->status,
ru.ru_utime.tv_sec + 1e-6 * ru.ru_utime.tv_usec,
ru.ru_stime.tv_sec + 1e-6 * ru.ru_stime.tv_usec
);
+ AZ(pthread_mutex_unlock(&p->mtx));
+
if (WIFEXITED(p->status) && WEXITSTATUS(p->status) == 0)
return (NULL);
#ifdef WCOREDUMP
@@ -193,8 +204,6 @@ process_thread(void *priv)
p->status, WTERMSIG(p->status), WEXITSTATUS(p->status));
#endif
- closefd(&p->fd_to);
-
return (NULL);
}
@@ -207,6 +216,8 @@ process_start(struct process *p)
int fdt[2];
CHECK_OBJ_NOTNULL(p, PROCESS_MAGIC);
+ if (p->hasthread)
+ vtc_log(p->vl, 0, "Already running, (-wait first)");
vtc_log(p->vl, 4, "CMD: %s", p->spec);
@@ -227,7 +238,6 @@ process_start(struct process *p)
}
p->pid = fork();
assert(p->pid >= 0);
- p->running = 1;
if (p->pid == 0) {
assert(dup2(fds[0], 0) == 0);
assert(dup2(out_fd, 1) == 1);
@@ -249,6 +259,7 @@ process_start(struct process *p)
closefd(&err_fd);
}
VSB_destroy(&cl);
+ p->hasthread = 1;
AZ(pthread_create(&p->tp, NULL, process_thread, p));
}
@@ -257,12 +268,14 @@ process_start(struct process *p)
*/
static void
-process_wait(const struct process *p)
+process_wait(struct process *p)
{
void *v;
- if (p->running && p->pid > 0)
+ if (p->hasthread) {
AZ(pthread_join(p->tp, &v));
+ p->hasthread = 0;
+ }
}
static void
@@ -278,14 +291,19 @@ process_run(struct process *p)
*/
static void
-process_kill(const struct process *p, const char *sig)
+process_kill(struct process *p, const char *sig)
{
int j = 0;
+ pid_t pid;
CHECK_OBJ_NOTNULL(p, PROCESS_MAGIC);
AN(sig);
+
+ AZ(pthread_mutex_lock(&p->mtx));
+ pid = p->pid;
+ AZ(pthread_mutex_unlock(&p->mtx));
- if (!p->running || p->pid <= 0)
+ if (pid <= 0)
vtc_log(p->vl, 0, "Cannot signal a non-running process");
if (!strcmp(sig, "TERM"))
@@ -299,19 +317,20 @@ process_kill(const struct process *p, const char *sig)
else
vtc_log(p->vl, 0, "Could not grok signal (%s)", sig);
- if (kill(-p->pid, j) < 0)
- vtc_log(p->vl, 0, "Failed to send signal %d (%s)", j, strerror(errno));
+ if (kill(-pid, j) < 0)
+ vtc_log(p->vl, 0, "Failed to send signal %d (%s)",
+ j, strerror(errno));
else
vtc_log(p->vl, 4, "Sent signal %d", j);
}
static inline void
-process_terminate(const struct process *p)
+process_terminate(struct process *p)
{
process_kill(p, "TERM");
sleep(1);
- if (p->running && p->pid > 0)
+ if (p->pid > 0)
process_kill(p, "KILL");
}
@@ -324,7 +343,7 @@ process_write(const struct process *p, const char *text)
{
int r, len;
- if (!p->running || p->pid <= 0)
+ if (!p->hasthread)
vtc_log(p->vl, 0, "Cannot write to a non-running process");
len = strlen(text);
@@ -339,7 +358,7 @@ static void
process_close(struct process *p)
{
- if (!p->running || p->pid <= 0)
+ if (!p->hasthread)
vtc_log(p->vl, 0, "Cannot close on a non-running process");
closefd(&p->fd_to);
@@ -403,8 +422,10 @@ cmd_process(CMD_ARGS)
if (av == NULL) {
/* Reset and free */
VTAILQ_FOREACH_SAFE(p, &processes, list, p2) {
- if (p->running && p->pid > 0)
+ if (p->pid > 0)
process_terminate(p);
+ if (p->hasthread)
+ process_wait(p);
VTAILQ_REMOVE(&processes, p, list);
process_delete(p);
}
More information about the varnish-commit
mailing list