Attachment 89600 Details for Bug 71738 – Revised path, adding pa_thread_join(). Also includes a log message.

[patch] Revised path, adding pa_thread_join(). Also includes a log message.

0001-Fix-undefined-behaviour-in-pulseaudio-start.patch (text/plain), 2.66 KB, created by Stefan Sperling on 2013-11-21 18:00:50 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Sperling

Created: 2013-11-21 18:00:50 UTC

Size: 2.66 KB

patch

obsolete

>From 3a024859a420780da37a7c1927e0356a25874174 Mon Sep 17 00:00:00 2001
>From: Stefan Sperling <stsp@openbsd.org>
>Date: Thu, 21 Nov 2013 18:47:42 +0100
>Subject: [PATCH] Fix undefined behaviour in pulseaudio --start.
>
>Don't call pthread_join() to join a thread from a different
>process than the thread was created in. Doing so can lead to
>undefined behaviour.
>
>On OpenBSD, the symptom was a pulseaudio process with a single
>thread waiting forever for other threads to join. Since that
>process also held the autospawn lock, starting new pulseaudio
>processes with --start kept failing. The problem was analyzed
>with help from Philip Guenther.
>
>This patch adds a pa_thread_free_nojoin() function which can
>be used to free resources for a thread without a join, as
>suggested by Tanu Kaskinen.
>
>See https://bugs.freedesktop.org/show_bug.cgi?id=71738
>---
> src/pulsecore/lock-autospawn.c | 9 ++++++++-
> src/pulsecore/thread-posix.c   | 7 +++++++
> src/pulsecore/thread.h         | 1 +
> 3 files changed, 16 insertions(+), 1 deletion(-)
>
>diff --git a/src/pulsecore/lock-autospawn.c b/src/pulsecore/lock-autospawn.c
>index 72806f8..82518db 100644
>--- a/src/pulsecore/lock-autospawn.c
>+++ b/src/pulsecore/lock-autospawn.c
>@@ -114,8 +114,15 @@ static void unref(bool after_fork) {
>     if (n_ref > 0)
>         return;
> 
>+    /* Join threads only in the process the new thread was created in
>+     * to avoid undefined behaviour.
>+     * POSIX.1-2008 XSH 2.9.2 Thread IDs: "applications should only assume
>+     * that thread IDs are usable and unique within a single process." */
>     if (thread) {
>-        pa_thread_free(thread);
>+        if (after_fork)
>+            pa_thread_free_nojoin(thread);
>+	else
>+            pa_thread_free(thread);
>         thread = NULL;
>     }
> 
>diff --git a/src/pulsecore/thread-posix.c b/src/pulsecore/thread-posix.c
>index b422ab0..5c19b43 100644
>--- a/src/pulsecore/thread-posix.c
>+++ b/src/pulsecore/thread-posix.c
>@@ -127,6 +127,13 @@ void pa_thread_free(pa_thread *t) {
>     pa_xfree(t);
> }
> 
>+void pa_thread_free_nojoin(pa_thread *t) {
>+    pa_assert(t);
>+
>+    pa_xfree(t->name);
>+    pa_xfree(t);
>+}
>+
> int pa_thread_join(pa_thread *t) {
>     pa_assert(t);
>     pa_assert(t->thread_func);
>diff --git a/src/pulsecore/thread.h b/src/pulsecore/thread.h
>index 9cabb89..a6e5f42 100644
>--- a/src/pulsecore/thread.h
>+++ b/src/pulsecore/thread.h
>@@ -39,6 +39,7 @@ typedef void (*pa_thread_func_t) (void *userdata);
> 
> pa_thread* pa_thread_new(const char *name, pa_thread_func_t thread_func, void *userdata);
> void pa_thread_free(pa_thread *t);
>+void pa_thread_free_nojoin(pa_thread *t);
> int pa_thread_join(pa_thread *t);
> int pa_thread_is_running(pa_thread *t);
> pa_thread *pa_thread_self(void);
>-- 
>1.8.4.3
>

Actions: View | Splinter Review

Attachments on bug 71738: 89406 | 89600

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.