diff mbox

[lttng-tools] Fix: thread exit vs futex wait/wakeup race

Message ID 1495060614-3414-1-git-send-email-mathieu.desnoyers@efficios.com
State Accepted, archived
Delegated to: Jérémie Galarneau
Headers show

Commit Message

Mathieu Desnoyers May 17, 2017, 10:36 p.m. UTC
relayd_live_stop performs, in this order:

        CMM_STORE_SHARED(live_dispatch_thread_exit, 1);   [A]
        futex_nto1_wake(&viewer_conn_queue.futex);        [B]

whereas thread_dispatcher does:

   while (!CMM_LOAD_SHARED(live_dispatch_thread_exit)) {  [1]

     [...]
     futex_nto1_prepare(&viewer_conn_queue.futex);        [2]
     [...]
     futex_nto1_wait(&viewer_conn_queue.futex);           [3]

Unfortunately, on the following sequence:

[1] [A] [B] [2] [3]

thread_dispatcher will end up hanging.

We need to move the live_dispatch_thread_exit load between "prepare" and
"wait" to fix this.

There are similar scenarios with relay_thread_dispatcher, and the
session daemon thread_dispatch_ust_registration, which are also fixed
here.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
---
 src/bin/lttng-relayd/live.c   | 6 +++++-
 src/bin/lttng-relayd/main.c   | 6 +++++-
 src/bin/lttng-sessiond/main.c | 6 +++++-
 3 files changed, 15 insertions(+), 3 deletions(-)

Comments

Jérémie Galarneau May 24, 2017, 3:12 p.m. UTC | #1
Merged in master, stable-2.10, stable-2.9, and stable-2.8.

Thanks!
Jérémie

On 17 May 2017 at 18:36, Mathieu Desnoyers
<mathieu.desnoyers at efficios.com> wrote:
> relayd_live_stop performs, in this order:
>
>         CMM_STORE_SHARED(live_dispatch_thread_exit, 1);   [A]
>         futex_nto1_wake(&viewer_conn_queue.futex);        [B]
>
> whereas thread_dispatcher does:
>
>    while (!CMM_LOAD_SHARED(live_dispatch_thread_exit)) {  [1]
>
>      [...]
>      futex_nto1_prepare(&viewer_conn_queue.futex);        [2]
>      [...]
>      futex_nto1_wait(&viewer_conn_queue.futex);           [3]
>
> Unfortunately, on the following sequence:
>
> [1] [A] [B] [2] [3]
>
> thread_dispatcher will end up hanging.
>
> We need to move the live_dispatch_thread_exit load between "prepare" and
> "wait" to fix this.
>
> There are similar scenarios with relay_thread_dispatcher, and the
> session daemon thread_dispatch_ust_registration, which are also fixed
> here.
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> ---
>  src/bin/lttng-relayd/live.c   | 6 +++++-
>  src/bin/lttng-relayd/main.c   | 6 +++++-
>  src/bin/lttng-sessiond/main.c | 6 +++++-
>  3 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/src/bin/lttng-relayd/live.c b/src/bin/lttng-relayd/live.c
> index 61acafe..f00270b 100644
> --- a/src/bin/lttng-relayd/live.c
> +++ b/src/bin/lttng-relayd/live.c
> @@ -654,12 +654,16 @@ void *thread_dispatcher(void *data)
>
>         health_code_update();
>
> -       while (!CMM_LOAD_SHARED(live_dispatch_thread_exit)) {
> +       for (;;) {
>                 health_code_update();
>
>                 /* Atomically prepare the queue futex */
>                 futex_nto1_prepare(&viewer_conn_queue.futex);
>
> +               if (CMM_LOAD_SHARED(live_dispatch_thread_exit)) {
> +                       break;
> +               }
> +
>                 do {
>                         health_code_update();
>
> diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c
> index 2fcc60a..0eb8e28 100644
> --- a/src/bin/lttng-relayd/main.c
> +++ b/src/bin/lttng-relayd/main.c
> @@ -977,12 +977,16 @@ static void *relay_thread_dispatcher(void *data)
>
>         health_code_update();
>
> -       while (!CMM_LOAD_SHARED(dispatch_thread_exit)) {
> +       for (;;) {
>                 health_code_update();
>
>                 /* Atomically prepare the queue futex */
>                 futex_nto1_prepare(&relay_conn_queue.futex);
>
> +               if (CMM_LOAD_SHARED(dispatch_thread_exit)) {
> +                       break;
> +               }
> +
>                 do {
>                         health_code_update();
>
> diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
> index 9b6f70c..e282a9c 100644
> --- a/src/bin/lttng-sessiond/main.c
> +++ b/src/bin/lttng-sessiond/main.c
> @@ -1934,12 +1934,16 @@ static void *thread_dispatch_ust_registration(void *data)
>
>         DBG("[thread] Dispatch UST command started");
>
> -       while (!CMM_LOAD_SHARED(dispatch_thread_exit)) {
> +       for (;;) {
>                 health_code_update();
>
>                 /* Atomically prepare the queue futex */
>                 futex_nto1_prepare(&ust_cmd_queue.futex);
>
> +               if (CMM_LOAD_SHARED(dispatch_thread_exit)) {
> +                       break;
> +               }
> +
>                 do {
>                         struct ust_app *app = NULL;
>                         ust_cmd = NULL;
> --
> 2.1.4
>
diff mbox

Patch

diff --git a/src/bin/lttng-relayd/live.c b/src/bin/lttng-relayd/live.c
index 61acafe..f00270b 100644
--- a/src/bin/lttng-relayd/live.c
+++ b/src/bin/lttng-relayd/live.c
@@ -654,12 +654,16 @@  void *thread_dispatcher(void *data)
 
 	health_code_update();
 
-	while (!CMM_LOAD_SHARED(live_dispatch_thread_exit)) {
+	for (;;) {
 		health_code_update();
 
 		/* Atomically prepare the queue futex */
 		futex_nto1_prepare(&viewer_conn_queue.futex);
 
+		if (CMM_LOAD_SHARED(live_dispatch_thread_exit)) {
+			break;
+		}
+
 		do {
 			health_code_update();
 
diff --git a/src/bin/lttng-relayd/main.c b/src/bin/lttng-relayd/main.c
index 2fcc60a..0eb8e28 100644
--- a/src/bin/lttng-relayd/main.c
+++ b/src/bin/lttng-relayd/main.c
@@ -977,12 +977,16 @@  static void *relay_thread_dispatcher(void *data)
 
 	health_code_update();
 
-	while (!CMM_LOAD_SHARED(dispatch_thread_exit)) {
+	for (;;) {
 		health_code_update();
 
 		/* Atomically prepare the queue futex */
 		futex_nto1_prepare(&relay_conn_queue.futex);
 
+		if (CMM_LOAD_SHARED(dispatch_thread_exit)) {
+			break;
+		}
+
 		do {
 			health_code_update();
 
diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index 9b6f70c..e282a9c 100644
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -1934,12 +1934,16 @@  static void *thread_dispatch_ust_registration(void *data)
 
 	DBG("[thread] Dispatch UST command started");
 
-	while (!CMM_LOAD_SHARED(dispatch_thread_exit)) {
+	for (;;) {
 		health_code_update();
 
 		/* Atomically prepare the queue futex */
 		futex_nto1_prepare(&ust_cmd_queue.futex);
 
+		if (CMM_LOAD_SHARED(dispatch_thread_exit)) {
+			break;
+		}
+
 		do {
 			struct ust_app *app = NULL;
 			ust_cmd = NULL;