DatabasePlugin: add method ReturnSong()

Allow the plugin to allocate the GetSong() return value.

13 files changed, 85 insertions, 14 deletions

diff --git a/src/DatabaseGlue.cxx b/src/DatabaseGlue.cxx
index 42812973d..88b25d868 100644
--- a/src/DatabaseGlue.cxx
+++ b/src/DatabaseGlue.cxx
@@ -132,6 +132,15 @@ db_get_song(const char *file)
 	return db->GetSong(file, NULL);
 }
 
+void
+db_return_song(struct song *song)
+{
+	assert(db != nullptr);
+	assert(song != nullptr);
+
+	db->ReturnSong(song);
+}
+
 bool
 db_save(GError **error_r)
 {
diff --git a/src/DatabasePlugin.hxx b/src/DatabasePlugin.hxx
index 37df7f654..a175b3cd9 100644
--- a/src/DatabasePlugin.hxx
+++ b/src/DatabasePlugin.hxx
@@ -85,7 +85,8 @@ public:
 	virtual void Close() {}
 
 	/**
-         * Look up a song (including tag data) in the database.
+         * Look up a song (including tag data) in the database.  When
+         * you don't need this anymore, call ReturnSong().
 	 *
 	 * @param uri_utf8 the URI of the song within the music
 	 * directory (UTF-8)
@@ -94,6 +95,12 @@ public:
 				     GError **error_r) const = 0;
 
 	/**
+	 * Mark the song object as "unused".  Call this on objects
+	 * returned by GetSong().
+	 */
+	virtual void ReturnSong(struct song *song) const = 0;
+
+	/**
 	 * Visit the selected entities.
 	 */
 	virtual bool Visit(const DatabaseSelection &selection,
diff --git a/src/command.c b/src/command.c
index 5bc6934c5..b4f8e7039 100644
--- a/src/command.c
+++ b/src/command.c
@@ -1326,6 +1326,7 @@ handle_sticker_song(struct client *client, int argc, char *argv[])
 		}
 
 		value = sticker_song_get_value(song, argv[4]);
+		db_return_song(song);
 		if (value == NULL) {
 			command_error(client, ACK_ERROR_NO_EXIST,
 				      "no such sticker");
@@ -1349,6 +1350,7 @@ handle_sticker_song(struct client *client, int argc, char *argv[])
 		}
 
 		sticker = sticker_song_get(song);
+		db_return_song(song);
 		if (sticker) {
 			sticker_print(client, sticker);
 			sticker_free(sticker);
@@ -1368,6 +1370,7 @@ handle_sticker_song(struct client *client, int argc, char *argv[])
 		}
 
 		ret = sticker_song_set_value(song, argv[4], argv[5]);
+		db_return_song(song);
 		if (!ret) {
 			command_error(client, ACK_ERROR_SYSTEM,
 				      "failed to set sticker value");
@@ -1391,6 +1394,7 @@ handle_sticker_song(struct client *client, int argc, char *argv[])
 		ret = argc == 4
 			? sticker_song_delete(song)
 			: sticker_song_delete_value(song, argv[4]);
+		db_return_song(song);
 		if (!ret) {
 			command_error(client, ACK_ERROR_SYSTEM,
 				      "no such sticker");
diff --git a/src/database.h b/src/database.h
index d8d86fa5d..e18663525 100644
--- a/src/database.h
+++ b/src/database.h
@@ -74,6 +74,10 @@ gcc_pure
 struct song *
 db_get_song(const char *file);
 
+gcc_nonnull(1)
+void
+db_return_song(struct song *song);
+
 /**
  * May only be used if db_is_simple() returns true.
  */
diff --git a/src/db/ProxyDatabasePlugin.cxx b/src/db/ProxyDatabasePlugin.cxx
index 26e7b4b84..e27ca8291 100644
--- a/src/db/ProxyDatabasePlugin.cxx
+++ b/src/db/ProxyDatabasePlugin.cxx
@@ -56,6 +56,8 @@ public:
 	virtual void Close() override;
 	virtual struct song *GetSong(const char *uri_utf8,
 				     GError **error_r) const override;
+	virtual void ReturnSong(struct song *song) const;
+
 	virtual bool Visit(const DatabaseSelection &selection,
 			   VisitDirectory visit_directory,
 			   VisitSong visit_song,
@@ -191,6 +193,16 @@ ProxyDatabase::GetSong(const char *uri, GError **error_r) const
 	return nullptr;
 }
 
+void
+ProxyDatabase::ReturnSong(struct song *song) const
+{
+	assert(song != nullptr);
+	assert(song_in_database(song));
+	assert(song_is_detached(song));
+
+	song_free(song);
+}
+
 static bool
 Visit(struct mpd_connection *connection, const char *uri,
       bool recursive, VisitDirectory visit_directory, VisitSong visit_song,
diff --git a/src/db/SimpleDatabasePlugin.cxx b/src/db/SimpleDatabasePlugin.cxx
index c1de70d3e..94318f4e9 100644
--- a/src/db/SimpleDatabasePlugin.cxx
+++ b/src/db/SimpleDatabasePlugin.cxx
@@ -184,6 +184,10 @@ SimpleDatabase::Open(GError **error_r)
 	root = directory_new_root();
 	mtime = 0;
 
+#ifndef NDEBUG
+	borrowed_song_count = 0;
+#endif
+
 	GError *error = NULL;
 	if (!Load(&error)) {
 		directory_free(root);
@@ -204,6 +208,7 @@ void
 SimpleDatabase::Close()
 {
 	assert(root != NULL);
+	assert(borrowed_song_count == 0);
 
 	directory_free(root);
 }
@@ -219,10 +224,25 @@ SimpleDatabase::GetSong(const char *uri, GError **error_r) const
 	if (song == NULL)
 		g_set_error(error_r, db_quark(), DB_NOT_FOUND,
 			    "No such song: %s", uri);
+#ifndef NDEBUG
+	else
+		++const_cast<unsigned &>(borrowed_song_count);
+#endif
 
 	return song;
 }
 
+void
+SimpleDatabase::ReturnSong(gcc_unused struct song *song) const
+{
+	assert(song != nullptr);
+
+#ifndef NDEBUG
+	assert(borrowed_song_count > 0);
+	--const_cast<unsigned &>(borrowed_song_count);
+#endif
+}
+
 G_GNUC_PURE
 const struct directory *
 SimpleDatabase::LookupDirectory(const char *uri) const
diff --git a/src/db/SimpleDatabasePlugin.hxx b/src/db/SimpleDatabasePlugin.hxx
index 7e3f5d2db..2ea5c4925 100644
--- a/src/db/SimpleDatabasePlugin.hxx
+++ b/src/db/SimpleDatabasePlugin.hxx
@@ -38,6 +38,10 @@ class SimpleDatabase : public Database {
 
 	time_t mtime;
 
+#ifndef NDEBUG
+	unsigned borrowed_song_count;
+#endif
+
 public:
 	gcc_pure
 	struct directory *GetRoot() {
@@ -61,6 +65,8 @@ public:
 
 	virtual struct song *GetSong(const char *uri_utf8,
 				     GError **error_r) const override;
+	virtual void ReturnSong(struct song *song) const;
+
 	virtual bool Visit(const DatabaseSelection &selection,
 			   VisitDirectory visit_directory,
 			   VisitSong visit_song,
diff --git a/src/playlist_edit.c b/src/playlist_edit.c
index 1dfe68daa..3cd737705 100644
--- a/src/playlist_edit.c
+++ b/src/playlist_edit.c
@@ -128,7 +128,12 @@ playlist_append_uri(struct playlist *playlist, struct player_control *pc,
 	if (song == NULL)
 		return PLAYLIST_RESULT_NO_SUCH_SONG;
 
-	return playlist_append_song(playlist, pc, song, added_id);
+	enum playlist_result result =
+		playlist_append_song(playlist, pc, song, added_id);
+	if (song_in_database(song))
+		db_return_song(song);
+
+	return result;
 }
 
 enum playlist_result
diff --git a/src/playlist_print.c b/src/playlist_print.c
index 204ce3582..b1b689893 100644
--- a/src/playlist_print.c
+++ b/src/playlist_print.c
@@ -127,6 +127,7 @@ spl_print(struct client *client, const char *name_utf8, bool detail,
 			struct song *song = db_get_song(temp);
 			if (song) {
 				song_print_info(client, song);
+				db_return_song(song);
 				wrote = true;
 			}
 		}
@@ -157,8 +158,7 @@ playlist_provider_print(struct client *client, const char *uri,
 		else
 			song_print_uri(client, song);
 
-		if (!song_in_database(song) || song_is_detached(song))
-			song_free(song);
+		song_free(song);
 	}
 
 	g_free(base_uri);
diff --git a/src/playlist_queue.c b/src/playlist_queue.c
index aada94984..8eb535dbd 100644
--- a/src/playlist_queue.c
+++ b/src/playlist_queue.c
@@ -41,8 +41,7 @@ playlist_load_into_queue(const char *uri, struct playlist_provider *source,
 	     ++i) {
 		if (i < start_index) {
 			/* skip songs before the start index */
-			if (!song_in_database(song))
-				song_free(song);
+			song_free(song);
 			continue;
 		}
 
@@ -51,9 +50,8 @@ playlist_load_into_queue(const char *uri, struct playlist_provider *source,
 			continue;
 
 		result = playlist_append_song(dest, pc, song, NULL);
+		song_free(song);
 		if (result != PLAYLIST_RESULT_SUCCESS) {
-			if (!song_in_database(song))
-				song_free(song);
 			g_free(base_uri);
 			return result;
 		}
diff --git a/src/playlist_song.c b/src/playlist_song.c
index 29efef2e3..3b8de6307 100644
--- a/src/playlist_song.c
+++ b/src/playlist_song.c
@@ -79,9 +79,7 @@ apply_song_metadata(struct song *dest, const struct song *src)
 		   (e.g. last track on a CUE file); fix it up here */
 		tmp->tag->time = dest->tag->time - src->start_ms / 1000;
 
-	if (!song_in_database(dest))
-		song_free(dest);
-
+	song_free(dest);
 	return tmp;
 }
 
@@ -97,10 +95,13 @@ playlist_check_load_song(const struct song *song, const char *uri, bool secure)
 		if (dest == NULL)
 			return NULL;
 	} else {
-		dest = db_get_song(uri);
-		if (dest == NULL)
+		struct song *tmp = db_get_song(uri);
+		if (tmp == NULL)
 			/* not found in database */
 			return NULL;
+
+		dest = song_dup_detached(tmp);
+		db_return_song(tmp);
 	}
 
 	return apply_song_metadata(dest, song);
diff --git a/src/queue_save.c b/src/queue_save.c
index a7c511c0e..0025e26f9 100644
--- a/src/queue_save.c
+++ b/src/queue_save.c
@@ -103,4 +103,7 @@ queue_load_song(FILE *fp, GString *buffer, const char *line,
 	}
 
 	queue_append(queue, song);
+
+	if (song_in_database(song))
+		db_return_song(song);
 }
diff --git a/src/stored_playlist.c b/src/stored_playlist.c
index 39ba2bac1..13defd177 100644
--- a/src/stored_playlist.c
+++ b/src/stored_playlist.c
@@ -496,7 +496,9 @@ spl_append_uri(const char *url, const char *utf8file, GError **error_r)
 			return false;
 		}
 
-		return spl_append_song(utf8file, song, error_r);
+		bool success = spl_append_song(utf8file, song, error_r);
+		db_return_song(song);
+		return success;
 	}
 }