directory: serialize song deletes from playlist during update
This makes the update code thread-safe and doesn't penalize the playlist code by complicating it with complicated and error-prone locks (and the associated overhead, not everybody has a thread-implementation as good as NPTL). The update task blocks during the delete; but the update task is a slow task anyways so we can block w/o people caring too much. This was also our only freeSong call site, so remove that function. Note that deleting entire directories is not fully thread-safe, yet; as their traversals are not yet locked.
This commit is contained in:
parent
2b965a5424
commit
7d8c9cc7e3
@ -31,6 +31,7 @@
|
|||||||
#include "song_save.h"
|
#include "song_save.h"
|
||||||
#include "main_notify.h"
|
#include "main_notify.h"
|
||||||
#include "dirvec.h"
|
#include "dirvec.h"
|
||||||
|
#include "condition.h"
|
||||||
|
|
||||||
#define DIRECTORY_DIR "directory: "
|
#define DIRECTORY_DIR "directory: "
|
||||||
#define DIRECTORY_MTIME "mtime: " /* DEPRECATED, noop-read-only */
|
#define DIRECTORY_MTIME "mtime: " /* DEPRECATED, noop-read-only */
|
||||||
@ -64,8 +65,13 @@ static time_t directory_dbModTime;
|
|||||||
static pthread_t update_thr;
|
static pthread_t update_thr;
|
||||||
|
|
||||||
static const int update_task_id_max = 1 << 15;
|
static const int update_task_id_max = 1 << 15;
|
||||||
|
|
||||||
static int update_task_id;
|
static int update_task_id;
|
||||||
|
|
||||||
|
static Song *delete;
|
||||||
|
|
||||||
|
static struct condition delete_cond;
|
||||||
|
|
||||||
static enum update_return
|
static enum update_return
|
||||||
addToDirectory(Directory * directory, const char *name);
|
addToDirectory(Directory * directory, const char *name);
|
||||||
|
|
||||||
@ -146,6 +152,16 @@ void reap_update_task(void)
|
|||||||
|
|
||||||
assert(pthread_equal(pthread_self(), main_task));
|
assert(pthread_equal(pthread_self(), main_task));
|
||||||
|
|
||||||
|
cond_enter(&delete_cond);
|
||||||
|
if (delete) {
|
||||||
|
char tmp[MPD_PATH_MAX];
|
||||||
|
LOG("removing: %s\n", get_song_url(tmp, delete));
|
||||||
|
deleteASongFromPlaylist(delete);
|
||||||
|
delete = NULL;
|
||||||
|
cond_signal_async(&delete_cond);
|
||||||
|
}
|
||||||
|
cond_leave(&delete_cond);
|
||||||
|
|
||||||
if (progress != UPDATE_PROGRESS_DONE)
|
if (progress != UPDATE_PROGRESS_DONE)
|
||||||
return;
|
return;
|
||||||
if (pthread_join(update_thr, (void **)&ret))
|
if (pthread_join(update_thr, (void **)&ret))
|
||||||
@ -217,10 +233,19 @@ static void freeDirectory(Directory * directory)
|
|||||||
|
|
||||||
static void delete_song(Directory *dir, Song *del)
|
static void delete_song(Directory *dir, Song *del)
|
||||||
{
|
{
|
||||||
char path_max_tmp[MPD_PATH_MAX]; /* wasteful */
|
/* first, prevent traversers in main task from getting this */
|
||||||
LOG("removing: %s\n", get_song_url(path_max_tmp, del));
|
|
||||||
songvec_delete(&dir->songs, del);
|
songvec_delete(&dir->songs, del);
|
||||||
freeSong(del); /* FIXME racy */
|
|
||||||
|
/* now take it out of the playlist (in the main_task) */
|
||||||
|
cond_enter(&delete_cond);
|
||||||
|
assert(!delete);
|
||||||
|
delete = del;
|
||||||
|
wakeup_main_task();
|
||||||
|
do { cond_wait(&delete_cond); } while (delete);
|
||||||
|
cond_leave(&delete_cond);
|
||||||
|
|
||||||
|
/* finally, all possible references gone, free it */
|
||||||
|
freeJustSong(del);
|
||||||
}
|
}
|
||||||
|
|
||||||
static void deleteEmptyDirectoriesInDirectory(Directory * directory)
|
static void deleteEmptyDirectoriesInDirectory(Directory * directory)
|
||||||
|
@ -79,12 +79,6 @@ Song *newSong(const char *url, Directory * parentDir)
|
|||||||
return song;
|
return song;
|
||||||
}
|
}
|
||||||
|
|
||||||
void freeSong(Song * song)
|
|
||||||
{
|
|
||||||
deleteASongFromPlaylist(song);
|
|
||||||
freeJustSong(song);
|
|
||||||
}
|
|
||||||
|
|
||||||
void freeJustSong(Song * song)
|
void freeJustSong(Song * song)
|
||||||
{
|
{
|
||||||
if (song->tag)
|
if (song->tag)
|
||||||
|
@ -42,8 +42,6 @@ song_alloc(const char *url, struct _Directory *parent);
|
|||||||
|
|
||||||
Song *newSong(const char *url, struct _Directory *parentDir);
|
Song *newSong(const char *url, struct _Directory *parentDir);
|
||||||
|
|
||||||
void freeSong(Song *);
|
|
||||||
|
|
||||||
void freeJustSong(Song *);
|
void freeJustSong(Song *);
|
||||||
|
|
||||||
int updateSongInfo(Song * song);
|
int updateSongInfo(Song * song);
|
||||||
|
Loading…
Reference in New Issue
Block a user