player_control: duplicate the song object

Make sure the player "owns" the next_song object, so nobody else can
free it.

5 files changed, 58 insertions, 7 deletions

diff --git a/src/player_control.c b/src/player_control.c
index 69357bf2..28134ccd 100644
--- a/src/player_control.c
+++ b/src/player_control.c
@@ -59,6 +59,9 @@ pc_new(unsigned buffer_chunks, unsigned int buffered_before_play)
 void
 pc_free(struct player_control *pc)
 {
+	if (pc->next_song != NULL)
+		song_free(pc->next_song);
+
 	g_cond_free(pc->cond);
 	g_mutex_free(pc->mutex);
 	g_free(pc);
@@ -284,6 +287,10 @@ pc_seek(struct player_control *pc, struct song *song, float seek_time)
 	assert(song != NULL);
 
 	player_lock(pc);
+
+	if (pc->next_song != NULL)
+		song_free(pc->next_song);
+
 	pc->next_song = song;
 	pc->seek_where = seek_time;
 	player_command_locked(pc, PLAYER_COMMAND_SEEK);
diff --git a/src/player_control.h b/src/player_control.h
index 8be6c469..c16dbcb9 100644
--- a/src/player_control.h
+++ b/src/player_control.h
@@ -123,7 +123,15 @@ struct player_control {
 	struct audio_format audio_format;
 	float total_time;
 	float elapsed_time;
+
+	/**
+	 * The next queued song.
+	 *
+	 * This is a duplicate, and must be freed when this attribute
+	 * is cleared.
+	 */
 	struct song *next_song;
+
 	double seek_where;
 	float cross_fade_seconds;
 	float mixramp_db;
@@ -198,6 +206,10 @@ player_lock_signal(struct player_control *pc)
 	player_unlock(pc);
 }
 
+/**
+ * @param song the song to be queued; the given instance will be owned
+ * and freed by the player
+ */
 void
 pc_play(struct player_control *pc, struct song *song);
 
@@ -261,12 +273,18 @@ pc_stop(struct player_control *pc);
 void
 pc_update_audio(struct player_control *pc);
 
+/**
+ * @param song the song to be queued; the given instance will be owned
+ * and freed by the player
+ */
 void
 pc_enqueue_song(struct player_control *pc, struct song *song);
 
 /**
  * Makes the player thread seek the specified song to a position.
  *
+ * @param song the song to be queued; the given instance will be owned
+ * and freed by the player
  * @return true on success, false on failure (e.g. if MPD isn't
  * playing currently)
  */
diff --git a/src/player_thread.c b/src/player_thread.c
index 1f48df66..d410984f 100644
--- a/src/player_thread.c
+++ b/src/player_thread.c
@@ -239,12 +239,18 @@ player_wait_for_decoder(struct player *player)
 	if (error != NULL) {
 		player_lock(pc);
 		pc_set_error(pc, PLAYER_ERROR_DECODER, error);
+
+		song_free(pc->next_song);
 		pc->next_song = NULL;
+
 		player_unlock(pc);
 
 		return false;
 	}
 
+	if (player->song != NULL)
+		song_free(player->song);
+
 	player->song = pc->next_song;
 	player->elapsed_time = 0.0;
 
@@ -486,6 +492,7 @@ static bool player_seek_decoder(struct player *player)
 			player->pipe = dc->pipe;
 		}
 
+		song_free(pc->next_song);
 		pc->next_song = NULL;
 		player->queued = false;
 	}
@@ -606,6 +613,7 @@ static void player_process_command(struct player *player)
 			player_lock(pc);
 		}
 
+		song_free(pc->next_song);
 		pc->next_song = NULL;
 		player->queued = false;
 		player_command_finished_locked(pc);
@@ -886,6 +894,8 @@ static void do_play(struct player_control *pc, struct decoder_control *dc)
 
 	player_dc_start(&player, player.pipe);
 	if (!player_wait_for_decoder(&player)) {
+		assert(player.song == NULL);
+
 		player_dc_stop(&player);
 		player_command_finished(pc);
 		music_pipe_free(player.pipe);
@@ -1048,10 +1058,14 @@ static void do_play(struct player_control *pc, struct decoder_control *dc)
 	if (player.cross_fade_tag != NULL)
 		tag_free(player.cross_fade_tag);
 
+	if (player.song != NULL)
+		song_free(player.song);
+
 	player_lock(pc);
 
 	if (player.queued) {
 		assert(pc->next_song != NULL);
+		song_free(pc->next_song);
 		pc->next_song = NULL;
 	}
 
@@ -1093,7 +1107,11 @@ player_task(gpointer arg)
 			/* fall through */
 
 		case PLAYER_COMMAND_PAUSE:
-			pc->next_song = NULL;
+			if (pc->next_song != NULL) {
+				song_free(pc->next_song);
+				pc->next_song = NULL;
+			}
+
 			player_command_finished_locked(pc);
 			break;
 
@@ -1134,7 +1152,11 @@ player_task(gpointer arg)
 			return NULL;
 
 		case PLAYER_COMMAND_CANCEL:
-			pc->next_song = NULL;
+			if (pc->next_song != NULL) {
+				song_free(pc->next_song);
+				pc->next_song = NULL;
+			}
+
 			player_command_finished_locked(pc);
 			break;
 
diff --git a/src/playlist.c b/src/playlist.c
index 4c95bc7c..4d4f0c1f 100644
--- a/src/playlist.c
+++ b/src/playlist.c
@@ -78,14 +78,15 @@ static void
 playlist_queue_song_order(struct playlist *playlist, struct player_control *pc,
 			  unsigned order)
 {
-	struct song *song;
 	char *uri;
 
 	assert(queue_valid_order(&playlist->queue, order));
 
 	playlist->queued = order;
 
-	song = queue_get_order(&playlist->queue, order);
+	struct song *song =
+		song_dup_detached(queue_get_order(&playlist->queue, order));
+
 	uri = song_get_uri(song);
 	g_debug("queue song %i:\"%s\"", playlist->queued, uri);
 	g_free(uri);
@@ -191,13 +192,13 @@ void
 playlist_play_order(struct playlist *playlist, struct player_control *pc,
 		    int orderNum)
 {
-	struct song *song;
 	char *uri;
 
 	playlist->playing = true;
 	playlist->queued = -1;
 
-	song = queue_get_order(&playlist->queue, orderNum);
+	struct song *song =
+		song_dup_detached(queue_get_order(&playlist->queue, orderNum));
 
 	uri = song_get_uri(song);
 	g_debug("play %i:\"%s\"", orderNum, uri);
diff --git a/src/playlist_control.c b/src/playlist_control.c
index 0dea7676..57cc428f 100644
--- a/src/playlist_control.c
+++ b/src/playlist_control.c
@@ -25,6 +25,7 @@
 #include "config.h"
 #include "playlist_internal.h"
 #include "player_control.h"
+#include "song.h"
 #include "idle.h"
 
 #include <glib.h>
@@ -239,7 +240,9 @@ playlist_seek_song(struct playlist *playlist, struct player_control *pc,
 		queued = NULL;
 	}
 
-	success = pc_seek(pc, queue_get_order(&playlist->queue, i), seek_time);
+	struct song *the_song =
+		song_dup_detached(queue_get_order(&playlist->queue, i));
+	success = pc_seek(pc, the_song, seek_time);
 	if (!success) {
 		playlist_update_queued_song(playlist, pc, queued);