From: Takashi Iwai <tiwai@suse.de>
Date: Tue, 17 May 2016 13:39:07 +0000 (+0200)
Subject: conf: Add thread-safe global tree reference
X-Git-Tag: v1.1.2~125
X-Git-Url: https://git.alsa-project.org/?a=commitdiff_plain;h=c9a0d7d601e8ab069f8745968c03c8470b24d20d;p=alsa-lib.git

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 087c05dc..5d293d58 100644
--- 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 f8b7a668..a5166114 100644
--- 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 8a5d530f..ae788431 100644
--- 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 5dc791c9..bac634ba 100644
--- 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 203e7a52..0d0d093d 100644
--- 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 0c89b8b9..4701b437 100644
--- 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 4405e68a..92798308 100644
--- 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 a25e4f79..b2534711 100644
--- 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 93d2455d..2072ceae 100644
--- 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;
 }
 
 /**