From e8ca429fbf0d01f55e99210d3de452cd7c686ac7 Mon Sep 17 00:00:00 2001
From: Michael Tuexen <tuexen@fh-muenster.de>
Date: Mon, 20 Jun 2016 20:27:38 +0200
Subject: [PATCH] Fix a bug in the error handling when blocking system call is
in place.
When a blocking system call is ongoing, the error message shown was
wrong. It was indicated that a blocking system call is on-going when
the program terminates. This is correct, but hides the error message
which describes why packetdrill is terminating.
Sponsored by: Netflix
---
gtests/net/packetdrill/code.c | 2 +-
gtests/net/packetdrill/run.c | 12 +++----
gtests/net/packetdrill/run.h | 2 +-
gtests/net/packetdrill/run_command.c | 2 +-
gtests/net/packetdrill/run_system_call.c | 40 +++++++++++++-----------
gtests/net/packetdrill/run_system_call.h | 3 +-
gtests/net/packetdrill/wire_server.c | 2 +-
7 files changed, 34 insertions(+), 29 deletions(-)
diff --git a/gtests/net/packetdrill/code.c b/gtests/net/packetdrill/code.c
index e0b89e91..e04a9ecb 100644
--- a/gtests/net/packetdrill/code.c
+++ b/gtests/net/packetdrill/code.c
@@ -619,7 +619,7 @@ void run_code_event(struct state *state, struct event *event,
error_out:
script_path = strdup(state->config->script_path);
- state_free(state);
+ state_free(state, 1);
die("%s:%d: runtime error in code: %s\n",
script_path, event->line_number, error);
free(script_path);
diff --git a/gtests/net/packetdrill/run.c b/gtests/net/packetdrill/run.c
index f033a318..0a7d3751 100644
--- a/gtests/net/packetdrill/run.c
+++ b/gtests/net/packetdrill/run.c
@@ -128,12 +128,12 @@ static void close_all_sockets(struct state *state)
}
}
-void state_free(struct state *state)
+void state_free(struct state *state, int about_to_die)
{
/* We have to stop the system call thread first, since it's using
* sockets that we want to close and reset.
*/
- syscalls_free(state, state->syscalls);
+ syscalls_free(state, state->syscalls, about_to_die);
/* Then we close the sockets and reset the connections, while
* we still have a netdev for injecting reset packets to free
@@ -402,7 +402,7 @@ static void run_local_packet_event(struct state *state, struct event *event,
fprintf(stderr, "%s", error);
free(error);
} else if (result == STATUS_ERR) {
- state_free(state);
+ state_free(state, 1);
die("%s\n", error);
}
}
@@ -553,7 +553,7 @@ void run_script(struct config *config, struct script *script)
while (1) {
if (get_next_event(state, &error)) {
- state_free(state);
+ state_free(state, 1);
die("%s\n", error);
}
event = state->event;
@@ -603,14 +603,14 @@ void run_script(struct config *config, struct script *script)
if (code_execute(state->code, &error)) {
char *script_path = strdup(state->config->script_path);
- state_free(state);
+ state_free(state, 1);
die("%s: error executing code: %s\n",
script_path, error);
free(script_path);
free(error);
}
- state_free(state);
+ state_free(state, 0);
DEBUGP("run_script: done running\n");
}
diff --git a/gtests/net/packetdrill/run.h b/gtests/net/packetdrill/run.h
index bb186fde..98169210 100644
--- a/gtests/net/packetdrill/run.h
+++ b/gtests/net/packetdrill/run.h
@@ -110,7 +110,7 @@ extern struct state *state_new(struct config *config,
struct netdev *netdev);
/* Free all run-time state for a test. */
-void state_free(struct state *state);
+void state_free(struct state *state, int about_to_die);
/* Grab the global lock for all global state. */
static inline void run_lock(struct state *state)
diff --git a/gtests/net/packetdrill/run_command.c b/gtests/net/packetdrill/run_command.c
index 7fbaa973..288ee464 100644
--- a/gtests/net/packetdrill/run_command.c
+++ b/gtests/net/packetdrill/run_command.c
@@ -50,7 +50,7 @@ void run_command_event(
error_out:
script_path = strdup(state->config->script_path);
- state_free(state);
+ state_free(state, 1);
die("%s:%d: error executing `%s` command: %s\n",
script_path, event->line_number,
command->command_line, error);
diff --git a/gtests/net/packetdrill/run_system_call.c b/gtests/net/packetdrill/run_system_call.c
index 93560daa..60fbd33a 100644
--- a/gtests/net/packetdrill/run_system_call.c
+++ b/gtests/net/packetdrill/run_system_call.c
@@ -6125,7 +6125,7 @@ static void invoke_system_call(
error_out:
script_path = strdup(state->config->script_path);
- state_free(state);
+ state_free(state, 1);
die("%s:%d: runtime error in %s call: %s\n",
script_path, event->line_number,
syscall->name, error);
@@ -6236,7 +6236,7 @@ static void enqueue_system_call(
error_out:
script_path = strdup(state->config->script_path);
- state_free(state);
+ state_free(state, 1);
die("%s:%d: runtime error in %s call: %s\n",
script_path, event->line_number,
syscall->name, error);
@@ -6378,31 +6378,35 @@ struct syscalls *syscalls_new(struct state *state)
return syscalls;
}
-void syscalls_free(struct state *state, struct syscalls *syscalls)
+void syscalls_free(struct state *state, struct syscalls *syscalls, int about_to_die)
{
+ int status;
+
/* Wait a bit for the thread to go idle. */
- if (await_idle_thread(state)) {
+ status = await_idle_thread(state);
+ if ((status == STATUS_ERR) && (about_to_die == 0)) {
die("%s:%d: runtime error: exiting while "
"a blocking system call is in progress\n",
state->config->script_path,
syscalls->event->line_number);
}
- /* Send a request to terminate the thread. */
- DEBUGP("main thread: signaling syscall thread to exit\n");
- syscalls->state = SYSCALL_EXITING;
- if (pthread_cond_signal(&syscalls->enqueued) != 0)
- die_perror("pthread_cond_signal");
-
- /* Release the lock briefly and wait for syscall thread to finish. */
- run_unlock(state);
- DEBUGP("main thread: unlocking, waiting for syscall thread exit\n");
- void *thread_result = NULL;
- if (pthread_join(syscalls->thread, &thread_result) != 0)
- die_perror("pthread_cancel");
- DEBUGP("main thread: joined syscall thread; relocking\n");
- run_lock(state);
+ if (status == STATUS_OK) {
+ /* Send a request to terminate the thread. */
+ DEBUGP("main thread: signaling syscall thread to exit\n");
+ syscalls->state = SYSCALL_EXITING;
+ if (pthread_cond_signal(&syscalls->enqueued) != 0)
+ die_perror("pthread_cond_signal");
+ /* Release the lock briefly and wait for syscall thread to finish. */
+ run_unlock(state);
+ DEBUGP("main thread: unlocking, waiting for syscall thread exit\n");
+ void *thread_result = NULL;
+ if (pthread_join(syscalls->thread, &thread_result) != 0)
+ die_perror("pthread_cancel");
+ DEBUGP("main thread: joined syscall thread; relocking\n");
+ run_lock(state);
+ }
if ((pthread_cond_destroy(&syscalls->idle) != 0) ||
(pthread_cond_destroy(&syscalls->enqueued) != 0) ||
(pthread_cond_destroy(&syscalls->dequeued) != 0)) {
diff --git a/gtests/net/packetdrill/run_system_call.h b/gtests/net/packetdrill/run_system_call.h
index 7dbd0887..ca30c5be 100644
--- a/gtests/net/packetdrill/run_system_call.h
+++ b/gtests/net/packetdrill/run_system_call.h
@@ -88,7 +88,8 @@ extern struct syscalls *syscalls_new(struct state *state);
/* Tear down a syscalls and free up the resources it has allocated. */
extern void syscalls_free(struct state *state,
- struct syscalls *syscalls);
+ struct syscalls *syscalls,
+ int about_to_die);
/* Execute the given system call event. The system call may be
* expected to block for a while, or it may be expected to return
diff --git a/gtests/net/packetdrill/wire_server.c b/gtests/net/packetdrill/wire_server.c
index ae144a9f..25e3d372 100644
--- a/gtests/net/packetdrill/wire_server.c
+++ b/gtests/net/packetdrill/wire_server.c
@@ -491,7 +491,7 @@ error_done:
fprintf(stderr, "%s\n", error);
if (wire_server->state != NULL)
- state_free(wire_server->state);
+ state_free(wire_server->state, 0);
DEBUGP("wire_server_thread: connection is done\n");
wire_server_free(wire_server);
--
GitLab