pcm: Better understandable locking code

The newly added locking code seems to have confused quite a few
people, as "thread_safe=1" may be considered as if the thread-safety
lock has to be turned on.  (It meant that the plugin _is_ thread-safe,
i.e. it needs no extra locking.)

For avoiding such a misunderstanding, this commit renames the relevant
pcm fields and give more comments to explain what is for what.
The former single pcm->thread_safe flag is now split to two boolean
flags, pcm->need_lock and pcm->lock_enabled.  It consumes a few more
bytes, but this would be (hopefully) better understandable.

No functional change by this commit.

Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index f8323999343e3ee7d73385f5e3cc44594b2ee095..cd87bc759ded95953e332b7e8d56b0f2d5b4185d 100644 (file)
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -2545,14 +2545,20 @@ int snd_pcm_new(snd_pcm_t **pcmp, snd_pcm_type_t type, const char *name,
        INIT_LIST_HEAD(&pcm->async_handlers);
 #ifdef THREAD_SAFE_API
        pthread_mutex_init(&pcm->lock, NULL);
+       /* use locking as default;
+        * each plugin may suppress this in its open call
+        */
+       pcm->need_lock = 1;
        {
-               static int default_thread_safe = -1;
-               if (default_thread_safe < 0) {
+               /* set lock_enabled field depending on $LIBASOUND_THREAD_SAFE */
+               static int do_lock_enable = -1; /* uninitialized */
+
+               /* evaluate env var only once at the first open for consistency */
+               if (do_lock_enable == -1) {
                        char *p = getenv("LIBASOUND_THREAD_SAFE");
-                       default_thread_safe = !p || *p != '0';
+                       do_lock_enable = !p || *p != '0';
                }
-               if (!default_thread_safe)
-                       pcm->thread_safe = -1; /* force to disable */
+               pcm->lock_enabled = do_lock_enable;
        }
 #endif
        *pcmp = pcm;

diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c
index 3a5634c1d39a024a3bf3000ce589a4a0decd89e8..56e88b6bf6c0adfb598f2acd35da693ae2bf6c53 100644 (file)
--- a/src/pcm/pcm_hw.c
+++ b/src/pcm/pcm_hw.c
@@ -1514,7 +1514,7 @@ int snd_pcm_hw_open_fd(snd_pcm_t **pcmp, const char *name,
        pcm->poll_events = info.stream == SND_PCM_STREAM_PLAYBACK ? POLLOUT : POLLIN;
        pcm->tstamp_type = tstamp_type;
 #ifdef THREAD_SAFE_API
-       pcm->thread_safe = 1;
+       pcm->need_lock = 0;     /* hw plugin is thread-safe */
 #endif
 
        ret = snd_pcm_hw_mmap_status(pcm);

diff --git a/src/pcm/pcm_local.h b/src/pcm/pcm_local.h
index bb7964d7833e3b8db3d8860428d05baadf13d7ff..bba2f15ac46326ab07a99b1d086ef57a29e78614 100644 (file)
--- a/src/pcm/pcm_local.h
+++ b/src/pcm/pcm_local.h
@@ -244,7 +244,12 @@ struct _snd_pcm {
        void *private_data;
        struct list_head async_handlers;
 #ifdef THREAD_SAFE_API
-       int thread_safe;
+       int need_lock;          /* true = this PCM (plugin) is thread-unsafe,
+                                * thus it needs a lock.
+                                */
+       int lock_enabled;       /* thread-safety lock is enabled on the system;
+                                * it's set depending on $LIBASOUND_THREAD_SAFE.
+                                */
        pthread_mutex_t lock;
 #endif
 };
@@ -1085,24 +1090,36 @@ static inline void sw_set_period_event(snd_pcm_sw_params_t *params, int val)
 #define PCMINABORT(pcm) (((pcm)->mode & SND_PCM_ABORT) != 0)
 
 #ifdef THREAD_SAFE_API
+/*
+ * __snd_pcm_lock() and __snd_pcm_unlock() are used to lock/unlock the plugin
+ * forcibly even if it's declared as thread-safe.  It's needed only for some
+ * codes that are thread-unsafe per design (e.g. snd_pcm_nonblock()).
+ *
+ * OTOH, snd_pcm_lock() and snd_pcm_unlock() are used to lock/unlock the plugin
+ * in normal situations.  They do lock/unlock only when the plugin is
+ * thread-unsafe.
+ *
+ * Both __snd_pcm_lock() and snd_pcm_lock() (and their unlocks) wouldn't do
+ * any action when the whole locking is disabled via $LIBASOUND_THREAD_SAFE=0.
+ */
 static inline void __snd_pcm_lock(snd_pcm_t *pcm)
 {
-       if (pcm->thread_safe >= 0)
+       if (pcm->lock_enabled)
                pthread_mutex_lock(&pcm->lock);
 }
 static inline void __snd_pcm_unlock(snd_pcm_t *pcm)
 {
-       if (pcm->thread_safe >= 0)
+       if (pcm->lock_enabled)
                pthread_mutex_unlock(&pcm->lock);
 }
 static inline void snd_pcm_lock(snd_pcm_t *pcm)
 {
-       if (!pcm->thread_safe)
+       if (pcm->lock_enabled && pcm->need_lock)
                pthread_mutex_lock(&pcm->lock);
 }
 static inline void snd_pcm_unlock(snd_pcm_t *pcm)
 {
-       if (!pcm->thread_safe)
+       if (pcm->lock_enabled && pcm->need_lock)
                pthread_mutex_unlock(&pcm->lock);
 }
 #else /* THREAD_SAFE_API */