osx_write: Be careful when dereferencing the private_data pointer.

[paraslash.git] / osx_write.c

diff --git a/osx_write.c b/osx_write.c
index 2c6fd0d2ef3867d5ad5e1e6d37db9e3b0083722c..d278d02c8ed5be97ffb0eb56b15825343c02e21c 100644 (file)
--- a/osx_write.c
+++ b/osx_write.c
@@ -43,9 +43,28 @@ struct private_osx_write_data {
        unsigned sample_format;
        /** Number of channels of the current audio stream. */
        unsigned channels;
-       /** Serializes access to buffer tree nodes. */
+       /**
+        * Serializes access to buffer tree nodes between the writer and
+        * the callback which runs in a different thread.
+        */
        int mutex;
-       /** The btr node of the callback. */
+       /**
+        * The btr node of the callback.
+        *
+        * Although access to the btr node is serialized between the writer and
+        * the callback via the above mutex, this does not stop other buffer
+        * tree nodes, for example the decoder, to race against the osx
+        * callback.
+        *
+        * However, since all operations on buffer tree nodes are local in the
+        * sense that they only affect one level in the buffer tree (i.e.
+        * parent or child nodes, but not the grandparent or the
+        * grandchildren), we may work around this problem by using another
+        * buffer tree node for the callback.
+        *
+        * The writer grabs the mutex in its post_select method and pushes down
+        * the buffers to the callback node.
+        */
        struct btr_node *callback_btrn;
 };
 
@@ -59,10 +78,14 @@ static OSStatus osx_callback(void *cb_arg, __a_unused AudioUnitRenderActionFlags
 {
        int i;
        struct writer_node *wn = cb_arg;
-       struct private_osx_write_data *powd = wn->private_data;
+       struct private_osx_write_data *powd;
        size_t samples_have, samples_want = 0;
 
+       powd = wn->private_data;
        mutex_lock(powd->mutex);
+       powd = wn->private_data;
+       if (!powd || !wn->btrn)
+               goto out;
        /*
         * We fill with zeros if no data was yet written and we do not have
         * enough to fill all buffers.
@@ -217,18 +240,18 @@ e0:
        return ret;
 }
 
-__malloc static void *osx_write_parse_config_or_die(const char *options)
+__malloc static void *osx_write_parse_config_or_die(int argc, char **argv)
 {
        struct osx_write_args_info *conf = para_calloc(sizeof(*conf));
 
        /* exits on errors */
-       osx_cmdline_parser_string(options, conf, "osx_write");
+       osx_write_cmdline_parser(argc, argv, conf);
        return conf;
 }
 
 static void osx_free_config(void *conf)
 {
-       osx_cmdline_parser_free(conf);
+       osx_write_cmdline_parser_free(conf);
 }
 
 static void osx_write_close(struct writer_node *wn)
@@ -266,7 +289,7 @@ static void osx_write_pre_select(struct sched *s, struct task *t)
        }
 
        mutex_lock(powd->mutex);
-       ret = btr_node_status(wn->btrn, wn->min_iqs, BTR_NT_LEAF);
+       ret = btr_node_status(wn->btrn, wn->min_iqs, BTR_NT_INTERNAL);
        if (ret < 0)
                drain_delay_nec = need_drain_delay(powd);
        mutex_unlock(powd->mutex);
@@ -298,22 +321,23 @@ static void osx_write_post_select(__a_unused struct sched *s, struct task *t)
                AudioOutputUnitStart(powd->audio_unit);
        }
        mutex_lock(powd->mutex);
-       btr_pushdown(btrn);
-       ret = btr_node_status(btrn, wn->min_iqs, BTR_NT_LEAF);
+       ret = btr_node_status(btrn, wn->min_iqs, BTR_NT_INTERNAL);
+       if (ret > 0)
+               btr_pushdown(btrn);
        if (ret < 0 && need_drain_delay(powd))
                ret = 0;
-       mutex_unlock(powd->mutex);
-
-       if (ret >= 0)
-               goto out;
+       if (ret >= 0) {
+               mutex_unlock(powd->mutex);
+               return;
+       }
        AudioOutputUnitStop(powd->audio_unit);
        AudioUnitUninitialize(powd->audio_unit);
        CloseComponent(powd->audio_unit);
        btr_remove_node(&powd->callback_btrn);
+       mutex_unlock(powd->mutex);
 remove_btrn:
        btr_remove_node(&wn->btrn);
        PARA_NOTICE_LOG("%s\n", para_strerror(-ret));
-out:
        t->error = ret;
 }
 
@@ -326,7 +350,7 @@ void osx_write_init(struct writer *w)
 {
        struct osx_write_args_info dummy;
 
-       osx_cmdline_parser_init(&dummy);
+       osx_write_cmdline_parser_init(&dummy);
        w->close = osx_write_close;
        w->pre_select = osx_write_pre_select;
        w->post_select = osx_write_post_select;
@@ -336,5 +360,5 @@ void osx_write_init(struct writer *w)
                .short_help = osx_write_args_info_help,
                .detailed_help = osx_write_args_info_detailed_help
        };
-       osx_cmdline_parser_free(&dummy);
+       osx_write_cmdline_parser_free(&dummy);
 }