conf: Add thread-safe global tree reference

Most of open functions in alsa-lib have the call pattern:
  snd_config_update();
  return snd_xxx_open(x, snd_config, ...);

This means that the toplevel config gets updated, and passed to a
local open function.  Although snd_config_update() itself has a
pthread mutex to be thread safe, the whole procedure above isn't
thread safe.  Namely, the global snd_config tree may be deleted and
recreated at any time while the open function is being processed.
This may lead to a data corruption and crash of the program.

For avoiding the corruption, this patch introduces a refcount to
config tree object.  A few new helper functions are introduced as
well:
- snd_config_update_ref() does update and take the refcount of the
  toplevel tree.   The obtained config tree has to be freed via
  snd_config_unref() below.
- snd_config_ref() and snd_config_unref() manage the refcount of the
  config object.  The latter eventually deletes the object when all
  references are gone.

Along with these additions, the caller of snd_config_update() and
snd_config global tree in alsa-lib are replaced with the new helpers.

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

diff --git a/include/conf.h b/include/conf.h
index 087c05dc6bcf9c1486b161e77944fb9791bb50fb..5d293d583fbb2085db567f2980f17dff0fb20f03 100644 (file)
--- a/include/conf.h
+++ b/include/conf.h
@@ -94,6 +94,10 @@ int snd_config_update_r(snd_config_t **top, snd_config_update_t **update, const
 int snd_config_update_free(snd_config_update_t *update);
 int snd_config_update_free_global(void);
 
+int snd_config_update_ref(snd_config_t **top);
+void snd_config_ref(snd_config_t *top);
+void snd_config_unref(snd_config_t *top);
+
 int snd_config_search(snd_config_t *config, const char *key,
                      snd_config_t **result);
 int snd_config_searchv(snd_config_t *config, 

diff --git a/src/conf.c b/src/conf.c
index f8b7a66865294ad7507afe995ee43ea4bb26d8b2..a516611427acda7406e4936b8cb5ef06f87f5b9a 100644 (file)
--- a/src/conf.c
+++ b/src/conf.c
@@ -434,6 +434,7 @@ static pthread_once_t snd_config_update_mutex_once = PTHREAD_ONCE_INIT;
 struct _snd_config {
        char *id;
        snd_config_type_t type;
+       int refcount; /* default = 0 */
        union {
                long integer;
                long long integer64;
@@ -1825,6 +1826,10 @@ int snd_config_remove(snd_config_t *config)
  * If the node is a compound node, its descendants (the whole subtree)
  * are deleted recursively.
  *
+ * The function is supposed to be called only for locally copied config
+ * trees.  For the global tree, take the reference via #snd_config_update_ref
+ * and free it via #snd_config_unref.
+ *
  * \par Conforming to:
  * LSB 3.2
  *
@@ -1833,6 +1838,10 @@ int snd_config_remove(snd_config_t *config)
 int snd_config_delete(snd_config_t *config)
 {
        assert(config);
+       if (config->refcount > 0) {
+               config->refcount--;
+               return 0;
+       }
        switch (config->type) {
        case SND_CONFIG_TYPE_COMPOUND:
        {
@@ -3833,6 +3842,8 @@ int snd_config_update_r(snd_config_t **_top, snd_config_update_t **_update, cons
  * \warning Whenever #snd_config is updated, all string pointers and
  * configuration node handles previously obtained from it may become
  * invalid.
+ * For safer operations, use #snd_config_update_ref and release the config
+ * via #snd_config_unref.
  *
  * \par Errors:
  * Any errors encountered when parsing the input or returned by hooks or
@@ -3851,6 +3862,74 @@ int snd_config_update(void)
        return err;
 }
 
+/**
+ * \brief Updates #snd_config and takes its reference.
+ * \return 0 if #snd_config was up to date, 1 if #snd_config was
+ *         updated, otherwise a negative error code.
+ *
+ * Unlike #snd_config_update, this function increases a reference counter
+ * so that the obtained tree won't be deleted until unreferenced by
+ * #snd_config_unref.
+ *
+ * This function is supposed to be thread-safe.
+ */
+int snd_config_update_ref(snd_config_t **top)
+{
+       int err;
+
+       if (top)
+               *top = NULL;
+       snd_config_lock();
+       err = snd_config_update_r(&snd_config, &snd_config_global_update, NULL);
+       if (err >= 0) {
+               if (snd_config) {
+                       if (top) {
+                               snd_config->refcount++;
+                               *top = snd_config;
+                       }
+               } else {
+                       err = -ENODEV;
+               }
+       }
+       snd_config_unlock();
+       return err;
+}
+
+/**
+ * \brief Take the reference of the config tree.
+ *
+ * Increases a reference counter of the given config tree.
+ *
+ * This function is supposed to be thread-safe.
+ */
+void snd_config_ref(snd_config_t *cfg)
+{
+       snd_config_lock();
+       if (cfg)
+               cfg->refcount++;
+       snd_config_unlock();
+}
+
+/**
+ * \brief Unreference the config tree.
+ *
+ * Decreases a reference counter of the given config tree, and eventually
+ * deletes the tree if all references are gone.  This is the counterpart of
+ * #snd_config_unref.
+ *
+ * Also, the config taken via #snd_config_update_ref must be unreferenced
+ * by this function, too.
+ *
+ * This function is supposed to be thread-safe.
+ */
+void snd_config_unref(snd_config_t *cfg)
+{
+       snd_config_lock();
+       if (cfg)
+               snd_config_delete(cfg);
+       snd_config_unlock();
+}
+
 /** 
  * \brief Frees a private update structure.
  * \param[in] update The private update structure to free.

diff --git a/src/control/control.c b/src/control/control.c
index 8a5d530f26749622c2ba6f0bd35a5f13bee6612f..ae7884313c63de4ef03c6562bc2fa55af6efb54b 100644 (file)
--- a/src/control/control.c
+++ b/src/control/control.c
@@ -968,12 +968,16 @@ static int snd_ctl_open_noupdate(snd_ctl_t **ctlp, snd_config_t *root, const cha
  */
 int snd_ctl_open(snd_ctl_t **ctlp, const char *name, int mode)
 {
+       snd_config_t *top;
        int err;
+
        assert(ctlp && name);
-       err = snd_config_update();
+       err = snd_config_update_ref(&top);
        if (err < 0)
                return err;
-       return snd_ctl_open_noupdate(ctlp, snd_config, name, mode);
+       err = snd_ctl_open_noupdate(ctlp, top, name, mode);
+       snd_config_unref(top);
+       return err;
 }
 
 /**

diff --git a/src/hwdep/hwdep.c b/src/hwdep/hwdep.c
index 5dc791c9918970b2b6bb8ec8c16d1c691fa8c3c1..bac634bae14abbbd2f3b8b65eb795051396cc8ff 100644 (file)
--- a/src/hwdep/hwdep.c
+++ b/src/hwdep/hwdep.c
@@ -168,12 +168,16 @@ static int snd_hwdep_open_noupdate(snd_hwdep_t **hwdep, snd_config_t *root, cons
  */
 int snd_hwdep_open(snd_hwdep_t **hwdep, const char *name, int mode)
 {
+       snd_config_t *top;
        int err;
+
        assert(hwdep && name);
-       err = snd_config_update();
+       err = snd_config_update_ref(&top);
        if (err < 0)
                return err;
-       return snd_hwdep_open_noupdate(hwdep, snd_config, name, mode);
+       err = snd_hwdep_open_noupdate(hwdep, top, name, mode);
+       snd_config_unref(top);
+       return err;
 }
 
 /**

diff --git a/src/pcm/pcm.c b/src/pcm/pcm.c
index 203e7a52491b408dc82b1121465c6e35daf96235..0d0d093deb49f2db7d207b29b728df372cce84bc 100644 (file)
--- a/src/pcm/pcm.c
+++ b/src/pcm/pcm.c
@@ -2288,12 +2288,16 @@ static int snd_pcm_open_noupdate(snd_pcm_t **pcmp, snd_config_t *root,
 int snd_pcm_open(snd_pcm_t **pcmp, const char *name, 
                 snd_pcm_stream_t stream, int mode)
 {
+       snd_config_t *top;
        int err;
+
        assert(pcmp && name);
-       err = snd_config_update();
+       err = snd_config_update_ref(&top);
        if (err < 0)
                return err;
-       return snd_pcm_open_noupdate(pcmp, snd_config, name, stream, mode, 0);
+       err = snd_pcm_open_noupdate(pcmp, top, name, stream, mode, 0);
+       snd_config_unref(top);
+       return err;
 }
 
 /**

diff --git a/src/rawmidi/rawmidi.c b/src/rawmidi/rawmidi.c
index 0c89b8b984b9e58049e42f0e423c369f61806a6f..4701b437535972b2ffdee875577bdf9318f1797c 100644 (file)
--- a/src/rawmidi/rawmidi.c
+++ b/src/rawmidi/rawmidi.c
@@ -305,12 +305,16 @@ static int snd_rawmidi_open_noupdate(snd_rawmidi_t **inputp, snd_rawmidi_t **out
 int snd_rawmidi_open(snd_rawmidi_t **inputp, snd_rawmidi_t **outputp,
                     const char *name, int mode)
 {
+       snd_config_t *top;
        int err;
+
        assert((inputp || outputp) && name);
-       err = snd_config_update();
+       err = snd_config_update_ref(&top);
        if (err < 0)
                return err;
-       return snd_rawmidi_open_noupdate(inputp, outputp, snd_config, name, mode);
+       err = snd_rawmidi_open_noupdate(inputp, outputp, top, name, mode);
+       snd_config_unref(top);
+       return err;
 }
 
 /**

diff --git a/src/seq/seq.c b/src/seq/seq.c
index 4405e68a7fe9cdf822d15a5311db04e3558a4b7e..92798308a3b08695d0268734ad0fbdec286d54ff 100644 (file)
--- a/src/seq/seq.c
+++ b/src/seq/seq.c
@@ -974,12 +974,16 @@ static int snd_seq_open_noupdate(snd_seq_t **seqp, snd_config_t *root,
 int snd_seq_open(snd_seq_t **seqp, const char *name, 
                 int streams, int mode)
 {
+       snd_config_t *top;
        int err;
+
        assert(seqp && name);
-       err = snd_config_update();
+       err = snd_config_update_ref(&top);
        if (err < 0)
                return err;
-       return snd_seq_open_noupdate(seqp, snd_config, name, streams, mode, 0);
+       err = snd_seq_open_noupdate(seqp, top, name, streams, mode, 0);
+       snd_config_unref(top);
+       return err;
 }
 
 /**

diff --git a/src/timer/timer.c b/src/timer/timer.c
index a25e4f797ce4e35a5112bc161245f3fe4e1b4eee..b25347117cba504ab284e05fd79535104299ae66 100644 (file)
--- a/src/timer/timer.c
+++ b/src/timer/timer.c
@@ -201,12 +201,16 @@ static int snd_timer_open_noupdate(snd_timer_t **timer, snd_config_t *root, cons
  */
 int snd_timer_open(snd_timer_t **timer, const char *name, int mode)
 {
+       snd_config_t *top;
        int err;
+
        assert(timer && name);
-       err = snd_config_update();
+       err = snd_config_update_ref(&top);
        if (err < 0)
                return err;
-       return snd_timer_open_noupdate(timer, snd_config, name, mode);
+       err = snd_timer_open_noupdate(timer, top, name, mode);
+       snd_config_unref(top);
+       return err;
 }
 
 /**

diff --git a/src/timer/timer_query.c b/src/timer/timer_query.c
index 93d2455d07fc33329d19e5552899ac4931968c28..2072ceaea3499e348a896bba05997fbd04ba065f 100644 (file)
--- a/src/timer/timer_query.c
+++ b/src/timer/timer_query.c
@@ -159,12 +159,16 @@ static int snd_timer_query_open_noupdate(snd_timer_query_t **timer, snd_config_t
  */
 int snd_timer_query_open(snd_timer_query_t **timer, const char *name, int mode)
 {
+       snd_config_t *top;
        int err;
+
        assert(timer && name);
-       err = snd_config_update();
+       err = snd_config_update_ref(&top);
        if (err < 0)
                return err;
-       return snd_timer_query_open_noupdate(timer, snd_config, name, mode);
+       err = snd_timer_query_open_noupdate(timer, top, name, mode);
+       snd_config_unref(top);
+       return err;
 }
 
 /**