Changeset 2fbd49c in mainline

Timestamp:

2022-09-22T09:01:32Z (20 months ago)

Author:

Jiri Svoboda <jiri@…>

Branches:

master, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

2139676

Parents:

a76e76b

git-author:

Jiri Svoboda <jiri@…> (2022-09-21 18:01:20)

git-committer:

Jiri Svoboda <jiri@…> (2022-09-22 09:01:32)

Message:

Audio synk needs locking

Without locking the list of sink connections, we are exposed
to a race between removing a connection at the end of playback
(typically while destroying a hound context) and audio device
event PCM_EVENT_FRAMES_PLAYED which causes audio mixing to occur
via audio_sink_mix_inputs(), causing hound to crash often at
the end of playback.

Location:

uspace/srv/audio/hound

Files:

• audio_sink.c (modified) (4 diffs)
• audio_sink.h (modified) (2 diffs)
• connection.c (modified) (2 diffs)
• hound.c (modified) (2 diffs)

uspace/srv/audio/hound/audio_sink.c

@@ -36 +36 @@
 #include <assert.h>
 #include <errno.h>
+#include <fibril_synch.h>
 #include <stdlib.h>
 #include <str.h>
@@ -64 +65 @@
                 return EINVAL;
         link_initialize(&sink->link);
+        fibril_mutex_initialize(&sink->lock);
         list_initialize(&sink->connections);
         sink->name = str_dup(name);
@@ -139 +141 @@
 
         pcm_format_silence(dest, size, &sink->format);
+        fibril_mutex_lock(&sink->lock);
         list_foreach(sink->connections, sink_link, connection_t, conn) {
                 const errno_t ret = connection_add_source_data(
@@ -147 +150 @@
                 }
         }
+        fibril_mutex_unlock(&sink->lock);
 }
 

uspace/srv/audio/hound/audio_sink.h

@@ -41 +41 @@
 #include <stdbool.h>
 #include <fibril.h>
+#include <fibril_synch.h>
 #include <pcm/format.h>
 
@@ -51 +52 @@
         /** Link in hound's sink list */
         link_t link;
+        /** Protect sink including list of connections */
+        fibril_mutex_t lock;
         /** List of all related connections */
         list_t connections;

uspace/srv/audio/hound/connection.c

@@ -61 +61 @@
                 conn->source = source;
                 list_append(&conn->source_link, &source->connections);
+                fibril_mutex_lock(&sink->lock);
                 list_append(&conn->sink_link, &sink->connections);
+                fibril_mutex_unlock(&sink->lock);
                 audio_sink_set_format(sink, audio_source_format(source));
                 if (source->connection_change)
@@ -83 +85 @@
         assert(!link_in_use(&connection->hound_link));
         list_remove(&connection->source_link);
+        fibril_mutex_lock(&connection->sink->lock);
         list_remove(&connection->sink_link);
+        fibril_mutex_unlock(&connection->sink->lock);
         if (connection->sink && connection->sink->connection_change)
                 connection->sink->connection_change(connection->sink, false);

uspace/srv/audio/hound/hound.c

@@ -126 +126 @@
         assert(fibril_mutex_is_locked(&hound->list_guard));
         log_verbose("Removing sink '%s'.", sink->name);
+        fibril_mutex_lock(&sink->lock);
         if (!list_empty(&sink->connections))
                 log_warning("Removing sink '%s' while still connected.", sink->name);
@@ -135 +136 @@
         }
         list_remove(&sink->link);
+        fibril_mutex_unlock(&sink->lock);
 }