From 8d6dab0928866350d981793cd9207d0fb284fb4d Mon Sep 17 00:00:00 2001 From: waker Date: Tue, 12 Apr 2011 22:16:58 +0200 Subject: intermediate fix to playlist/streamer/plugevent mutex race conditions --- main.c | 34 +++++++++++---------- playlist.c | 72 +++++++++++++++++++-------------------------- plmeta.c | 46 +++++++++++++++++++++++++++-- plugins.c | 24 +++++++++++---- plugins/gtkui/ddblistview.c | 11 ++++++- plugins/gtkui/gtkui.c | 14 +++++++-- plugins/gtkui/search.c | 8 +++++ plugins/gtkui/search.h | 3 ++ streamer.c | 6 ++-- 9 files changed, 147 insertions(+), 71 deletions(-) diff --git a/main.c b/main.c index 3cdf60a9..1668eaee 100644 --- a/main.c +++ b/main.c @@ -234,29 +234,31 @@ server_exec_command_line (const char *cmdline, int len, char *sendback, int sbsi int curr_plt = plt_get_curr (); if (!queue) { pl_clear (); + plug_trigger_event_playlistchanged (); pl_reset_cursor (); } if (parg < pend) { + printf ("query to add files\n"); deadbeef->pl_add_files_begin (curr_plt); - } - while (parg < pend) { - char resolved[PATH_MAX]; - const char *pname; - if (realpath (parg, resolved)) { - pname = resolved; - } - else { - pname = parg; - } - if (deadbeef->pl_add_dir (pname, NULL, NULL) < 0) { - if (deadbeef->pl_add_file (pname, NULL, NULL) < 0) { - fprintf (stderr, "failed to add file or folder %s\n", pname); + while (parg < pend) { + char resolved[PATH_MAX]; + const char *pname; + if (realpath (parg, resolved)) { + pname = resolved; } + else { + pname = parg; + } + if (deadbeef->pl_add_dir (pname, NULL, NULL) < 0) { + if (deadbeef->pl_add_file (pname, NULL, NULL) < 0) { + fprintf (stderr, "failed to add file or folder %s\n", pname); + } + } + parg += strlen (parg); + parg++; } - parg += strlen (parg); - parg++; + deadbeef->pl_add_files_end (); } - deadbeef->pl_add_files_end (); messagepump_push (M_PLAYLIST_REFRESH, 0, 0, 0); if (!queue) { messagepump_push (M_PLAY_CURRENT, 0, 1, 0); diff --git a/playlist.c b/playlist.c index 1c8dd150..59b91b71 100644 --- a/playlist.c +++ b/playlist.c @@ -141,10 +141,14 @@ pl_free (void) { #if DEBUG_LOCKING int plt_lock_cnt = 0; #endif +pthread_t plt_lock_tid = 0; void plt_lock (void) { + pl_lock (); + return; #if !DISABLE_LOCKING mutex_lock (mutex_plt); + plt_lock_tid = pthread_self(); #if DEBUG_LOCKING plt_lock_cnt++; printf ("cnt: %d\n", plt_lock_cnt); @@ -154,6 +158,8 @@ plt_lock (void) { void plt_unlock (void) { + pl_unlock (); + return; #if !DISABLE_LOCKING mutex_unlock (mutex_plt); #if DEBUG_LOCKING @@ -166,10 +172,13 @@ plt_unlock (void) { #if DEBUG_LOCKING int pl_lock_cnt = 0; #endif +pthread_t pl_lock_tid = 0; void pl_lock (void) { #if !DISABLE_LOCKING mutex_lock (mutex); + pl_lock_tid = pthread_self(); + #if DEBUG_LOCKING pl_lock_cnt++; printf ("pcnt: %d\n", pl_lock_cnt); @@ -1469,7 +1478,6 @@ pl_insert_file (playItem_t *after, const char *fname, int *pabort, int (*cb)(pla playItem_t *it = pl_item_alloc_init (fname, NULL); pl_replace_meta (it, ":FILETYPE", "content"); - pl_add_meta (it, "title", NULL); after = plt_insert_item (addfiles_playlist ? addfiles_playlist : playlist, after, it); pl_item_unref (it); return after; @@ -1489,6 +1497,7 @@ pl_insert_file (playItem_t *after, const char *fname, int *pabort, int (*cb)(pla if (!strcasecmp (exts[e], eol)) { playItem_t *inserted = (playItem_t *)decoders[i]->insert (DB_PLAYITEM (after), fname); if (inserted != NULL) { + printf ("inserted %s\n", fname); if (cb && cb (inserted, user_data) < 0) { *pabort = 1; } @@ -1623,6 +1632,7 @@ pl_add_files_begin (int plt) { void pl_add_files_end (void) { + printf ("end adding to playlist %s\n", addfiles_playlist->title); addfiles_playlist = NULL; } @@ -2568,32 +2578,6 @@ pl_reshuffle (playItem_t **ppmin, playItem_t **ppmax) { plt_reshuffle (playlist, ppmin, ppmax); } -void -pl_delete_all_meta (playItem_t *it) { - LOCK; - DB_metaInfo_t *m = it->meta; - DB_metaInfo_t *prev = NULL; - while (m) { - DB_metaInfo_t *next = m->next; - if (m->key[0] == ':') { - prev = m; - } - else { - if (prev) { - prev->next = next; - } - else { - it->meta = next; - } - metacache_remove_string (m->key); - metacache_remove_string (m->value); - free (m); - } - m = next; - } - UNLOCK; -} - void pl_set_item_duration (playItem_t *it, float duration) { GLOBAL_LOCK; @@ -2873,20 +2857,25 @@ pl_format_title_int (const char *escape_chars, playItem_t *it, int idx, char *s, meta = pl_find_meta (it, "title"); if (!meta) { const char *f = pl_find_meta (it, ":URI"); - const char *start = strrchr (f, '/'); - if (start) { - start++; - } - else { - start = f; - } - const char *end = strrchr (start, '.'); - if (end) { - int n = end-start; - n = min (end-start, sizeof (dirname)-1); - strncpy (dirname, start, n); - dirname[n] = 0; - meta = dirname; + if (f) { + const char *start = strrchr (f, '/'); + if (start) { + start++; + } + else { + start = f; + } + const char *end = strrchr (start, '.'); + if (end) { + int n = end-start; + n = min (end-start, sizeof (dirname)-1); + strncpy (dirname, start, n); + dirname[n] = 0; + meta = dirname; + } + else { + meta = "?"; + } } else { meta = "?"; @@ -3416,6 +3405,7 @@ pl_move_items (int iter, int plt_from, playItem_t *drop_before, uint32_t *indexe void pl_copy_items (int iter, int plt_from, playItem_t *before, uint32_t *indices, int cnt) { pl_lock (); + playlist_t *from = playlists_head; playlist_t *to = plt_get_curr_ptr (); diff --git a/plmeta.c b/plmeta.c index ed1329e4..6b287637 100644 --- a/plmeta.c +++ b/plmeta.c @@ -86,6 +86,7 @@ pl_add_meta (playItem_t *it, const char *key, const char *value) { void pl_append_meta (playItem_t *it, const char *key, const char *value) { + pl_lock (); const char *old = pl_find_meta (it, key); size_t newlen = strlen (value); if (!old) { @@ -107,6 +108,7 @@ pl_append_meta (playItem_t *it, const char *key, const char *value) { } if (len == newlen && !memcmp (str, value, len)) { + pl_unlock (); return; } @@ -117,6 +119,7 @@ pl_append_meta (playItem_t *it, const char *key, const char *value) { snprintf (out, sz, "%s\n%s", old, value); pl_replace_meta (it, key, out); } + pl_unlock (); } void @@ -158,6 +161,7 @@ pl_set_meta_float (playItem_t *it, const char *key, float value) { void pl_delete_meta (playItem_t *it, const char *key) { + pl_lock (); DB_metaInfo_t *prev = NULL; DB_metaInfo_t *m = it->meta; while (m) { @@ -176,30 +180,40 @@ pl_delete_meta (playItem_t *it, const char *key) { prev = m; m = m->next; } + pl_unlock (); } const char * pl_find_meta (playItem_t *it, const char *key) { + pl_lock (); DB_metaInfo_t *m = it->meta; while (m) { if (!strcasecmp (key, m->key)) { + pl_unlock (); return m->value; } m = m->next; } + pl_unlock (); return NULL; } int pl_find_meta_int (playItem_t *it, const char *key, int def) { + pl_lock (); const char *val = pl_find_meta (it, key); - return val ? atoi (val) : def; + int res = val ? atoi (val) : def; + pl_unlock (); + return res; } float pl_find_meta_float (playItem_t *it, const char *key, float def) { + pl_lock (); const char *val = pl_find_meta (it, key); - return val ? atof (val) : def; + float res = val ? atof (val) : def; + pl_unlock (); + return res; } DB_metaInfo_t * @@ -209,6 +223,7 @@ pl_get_metadata_head (playItem_t *it) { void pl_delete_metadata (playItem_t *it, DB_metaInfo_t *meta) { + pl_lock (); DB_metaInfo_t *prev = NULL; DB_metaInfo_t *m = it->meta; while (m) { @@ -227,6 +242,31 @@ pl_delete_metadata (playItem_t *it, DB_metaInfo_t *meta) { prev = m; m = m->next; } + pl_unlock (); } - +void +pl_delete_all_meta (playItem_t *it) { + LOCK; + DB_metaInfo_t *m = it->meta; + DB_metaInfo_t *prev = NULL; + while (m) { + DB_metaInfo_t *next = m->next; + if (m->key[0] == ':') { + prev = m; + } + else { + if (prev) { + prev->next = next; + } + else { + it->meta = next; + } + metacache_remove_string (m->key); + metacache_remove_string (m->value); + free (m); + } + m = next; + } + UNLOCK; +} diff --git a/plugins.c b/plugins.c index 09f83c43..74ce90d1 100644 --- a/plugins.c +++ b/plugins.c @@ -395,11 +395,23 @@ typedef struct plugin_s { plugin_t *plugins; plugin_t *plugins_tail; +void +plug_lock (void) { +// mutex_lock (mutex); + pl_lock (); +} + +void +plug_unlock (void) { +// mutex_unlock (mutex); + pl_unlock (); +} + void plug_ev_subscribe (DB_plugin_t *plugin, int ev, DB_callback_t callback, uintptr_t data) { assert (ev < DB_EV_MAX && ev >= 0); int i; - mutex_lock (mutex); + plug_lock (); for (i = 0; i < MAX_HANDLERS; i++) { if (!handlers[ev][i].plugin) { handlers[ev][i].plugin = plugin; @@ -408,7 +420,7 @@ plug_ev_subscribe (DB_plugin_t *plugin, int ev, DB_callback_t callback, uintptr_ break; } } - mutex_unlock (mutex); + plug_unlock (); if (i == MAX_HANDLERS) { trace ("failed to subscribe plugin %s to event %d (too many event handlers)\n", plugin->name, ev); } @@ -417,7 +429,7 @@ plug_ev_subscribe (DB_plugin_t *plugin, int ev, DB_callback_t callback, uintptr_ void plug_ev_unsubscribe (DB_plugin_t *plugin, int ev, DB_callback_t callback, uintptr_t data) { assert (ev < DB_EV_MAX && ev >= 0); - mutex_lock (mutex); + plug_lock (); for (int i = 0; i < MAX_HANDLERS; i++) { if (handlers[ev][i].plugin == plugin) { handlers[ev][i].plugin = NULL; @@ -426,7 +438,7 @@ plug_ev_unsubscribe (DB_plugin_t *plugin, int ev, DB_callback_t callback, uintpt break; } } - mutex_unlock (mutex); + plug_unlock (); } float @@ -470,7 +482,7 @@ plug_event_call (DB_event_t *ev) { } ev->time = time (NULL); // printf ("plug_event_call enter %d\n", ev->event); - mutex_lock (mutex); + plug_lock (); for (int i = 0; i < MAX_HANDLERS; i++) { if (handlers[ev->event][i].plugin) { @@ -482,7 +494,7 @@ plug_event_call (DB_event_t *ev) { // pl_save_current (); // } - mutex_unlock (mutex); + plug_unlock (); // printf ("plug_event_call leave %d\n", ev->event); } diff --git a/plugins/gtkui/ddblistview.c b/plugins/gtkui/ddblistview.c index 8c27a971..18ccf4a4 100644 --- a/plugins/gtkui/ddblistview.c +++ b/plugins/gtkui/ddblistview.c @@ -591,6 +591,7 @@ ddb_listview_list_render (DdbListview *listview, int x, int y, int w, int h) { deadbeef->pl_lock (); // find 1st group DdbListviewGroup *grp = listview->groups; + printf ("starting to render listview, groups=%p, num_items=%d\n", grp, grp?grp->num_items : 0); int grp_y = 0; while (grp && grp_y + grp->height < y + listview->scrollpos) { grp_y += grp->height; @@ -724,7 +725,7 @@ ddb_listview_vscroll_event (GtkWidget *widget, { DdbListview *ps = DDB_LISTVIEW (g_object_get_data (G_OBJECT (widget), "owner")); GdkEventScroll *ev = (GdkEventScroll*)event; - GtkWidget *range = ps->scrollbar;; + GtkWidget *range = ps->scrollbar; GtkWidget *list = ps->list; // pass event to scrollbar int newscroll = gtk_range_get_value (GTK_RANGE (range)); @@ -2871,6 +2872,7 @@ ddb_listview_build_groups (DdbListview *listview) { memset (grp, 0, sizeof (DdbListviewGroup)); grp->head = it; grp->num_items = listview->binding->count (); + printf ("numitems: %d\n", grp->num_items); listview->grouptitle_height = 0; grp->height = listview->grouptitle_height + grp->num_items * listview->rowheight; // if (grp->height < min_height) { @@ -2913,8 +2915,15 @@ ddb_listview_build_groups (DdbListview *listview) { } listview->fullheight += grp->height; } + if (!listview->groups) { + printf ("empty!\n"); + } + else { + printf ("groupsize: %d!\n", listview->groups->num_items); + } deadbeef->pl_unlock (); } + void ddb_listview_set_vscroll (DdbListview *listview, gboolean scroll) { gtk_range_set_value (GTK_RANGE (listview->scrollbar), scroll); diff --git a/plugins/gtkui/gtkui.c b/plugins/gtkui/gtkui.c index 78069634..a43bf409 100644 --- a/plugins/gtkui/gtkui.c +++ b/plugins/gtkui/gtkui.c @@ -347,6 +347,7 @@ void redraw_queued_tracks (DdbListview *pl, int list) { DB_playItem_t *it; int idx = 0; + deadbeef->pl_lock (); for (it = deadbeef->pl_get_first (PL_MAIN); it; idx++) { if (deadbeef->pl_playqueue_test (it) != -1) { ddb_listview_draw_row (pl, idx, (DdbListviewIter)it); @@ -355,6 +356,7 @@ redraw_queued_tracks (DdbListview *pl, int list) { deadbeef->pl_item_unref (it); it = next; } + deadbeef->pl_unlock (); } static gboolean @@ -490,11 +492,16 @@ playlistchanged_cb (gpointer none) { void gtkui_playlist_changed (void) { + DdbListview *pl = DDB_LISTVIEW (lookup_widget (mainwin, "playlist")); + ddb_listview_build_groups (pl); + search_rebuild_groups (); + g_idle_add (playlistchanged_cb, NULL); } static int gtkui_on_playlistchanged (DB_event_t *ev, uintptr_t data) { + printf ("gtkui_on_playlistchanged\n"); gtkui_playlist_changed (); return 0; } @@ -528,6 +535,10 @@ playlistswitch_cb (gpointer none) { static int gtkui_on_playlistswitch (DB_event_t *ev, uintptr_t data) { + printf ("gtkui_on_playlistchanged\n"); + DdbListview *pl = DDB_LISTVIEW (lookup_widget (mainwin, "playlist")); + ddb_listview_build_groups (pl); + search_rebuild_groups (); g_idle_add (playlistswitch_cb, NULL); return 0; } @@ -1108,8 +1119,7 @@ gtkui_set_progress_text_idle (gpointer data) { gboolean gtkui_progress_hide_idle (gpointer data) { progress_hide (); - deadbeef->sendmessage (M_PLAYLIST_REFRESH, 0, 0, 0); - //playlist_refresh (); + //deadbeef->sendmessage (M_PLAYLIST_REFRESH, 0, 0, 0); return FALSE; } diff --git a/plugins/gtkui/search.c b/plugins/gtkui/search.c index 4509d3d5..cde5ceea 100644 --- a/plugins/gtkui/search.c +++ b/plugins/gtkui/search.c @@ -92,6 +92,14 @@ search_refresh (void) { } } +void +search_rebuild_groups (void) { + if (searchwin) { + GtkWidget *pl = lookup_widget (searchwin, "searchlist"); + ddb_listview_build_groups (DDB_LISTVIEW (pl)); + } +} + ///////// searchwin header handlers gboolean diff --git a/plugins/gtkui/search.h b/plugins/gtkui/search.h index 98216d5c..ddfe2844 100644 --- a/plugins/gtkui/search.h +++ b/plugins/gtkui/search.h @@ -36,4 +36,7 @@ search_get_idx (DdbListviewIter it); void search_playlist_init (GtkWidget *widget); +void +search_rebuild_groups (void); + #endif // __SEARCH_H diff --git a/streamer.c b/streamer.c index 567f9e95..e321a882 100644 --- a/streamer.c +++ b/streamer.c @@ -112,12 +112,14 @@ static DB_FILE *streamer_file; void streamer_lock (void) { - mutex_lock (mutex); +// mutex_lock (mutex); + pl_lock (); } void streamer_unlock (void) { - mutex_unlock (mutex); +// mutex_unlock (mutex); + pl_unlock (); } static void -- cgit v1.2.3