[vlc-commits] [Git][videolan/vlc][master] 4 commits: dbus: dbus_tracklist: add missing inline qualifier
Jean-Baptiste Kempf (@jbk)
gitlab at videolan.org
Tue Jul 11 09:37:55 UTC 2023
Jean-Baptiste Kempf pushed to branch master at VideoLAN / VLC
Commits:
8ce9af05 by Alexandre Janniaux at 2023-07-11T09:05:08+00:00
dbus: dbus_tracklist: add missing inline qualifier
Those functions are defined in a header file which will not use them
directly. `static` only will imply that the function will be used in the
file including it or the header itself. `static inline` is much more
suitable and will remove the unused function warning.
- - - - -
f9b56da3 by Alexandre Janniaux at 2023-07-11T09:05:08+00:00
dbus: dbus: allow duplicated playlist event
This commit fixes a memory leak on playlist event.
After MR !3189 [^1], the leak was fixed when closing the interface
before the events are processed, but not when the events were being
processed and the ownership moved to the dbus thread.
When a second tracklist (append or remove) event was queued to the dbus
thread, it detected that an existing event was already there and
discarded the event without destroying it, despite the ownership being
transferred.
It is necessary to check whether the event was transferred or not and
release it if not, which will be done in a following commit, but the
tracklist events are gathered by the event processing code and event
type duplicates don't have the same information and shouldn't be
discarded first, which solves the following root leak:
==81939==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x7fd01ced85cf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x7fd00d6a3129 in tracklist_append_event_create ../../modules/control/dbus/dbus_tracklist.c:41
#2 0x7fd00d6b1f98 in playlist_on_items_added ../../modules/control/dbus/dbus.c:1063
#3 0x7fd01c03e071 in vlc_playlist_ItemsInserted ../../src/playlist/content.c:76
#4 0x7fd01c045b3e in vlc_playlist_Expand ../../src/playlist/content.c:382
#5 0x7fd01c0537e8 in vlc_playlist_ExpandItem ../../src/playlist/preparse.c:59
#6 0x7fd01c053942 in vlc_playlist_ExpandItemFromNode ../../src/playlist/preparse.c:76
#7 0x7fd01c05397f in on_subtree_added ../../src/playlist/preparse.c:87
#8 0x7fd01c070088 in OnParserSubtreeAdded ../../src/preparser/preparser.c:171
#9 0x7fd01c08848b in input_item_parser_InputEvent ../../src/input/item.c:1402
[^1]: https://code.videolan.org/videolan/vlc/-/merge_requests/3189
Refs #27780
Fixes #28307
- - - - -
3db58145 by Alexandre Janniaux at 2023-07-11T09:05:08+00:00
dbus: dbus: delete event on error
When a second tracklist (append or remove) event was queued to the dbus
thread, it detected that an existing event was already there and
discarded the event without destroying it, despite the ownership being
transferred.
By checking whether the event was transferred or not and release it if
not, we don't risk leaking the event structures and the underlying items
hold by them.
No functional changes since those events were not discarded anymore.
- - - - -
a168eb01 by Alexandre Janniaux at 2023-07-11T09:05:08+00:00
dbus: dbus: refactor release of events
Tracklist events were aggregated by the processing code into it's own
linked-list which was then used to drpo the events.
The code was different from how events are released when closing the
application with events that were not sent to the dbus thread. This
commit unify the way they are released by duplicating the existing
release code matching the signal type and dropping them from the
ProcessEvents function instead.
- - - - -
3 changed files:
- modules/control/dbus/dbus.c
- modules/control/dbus/dbus_tracklist.c
- modules/control/dbus/dbus_tracklist.h
Changes:
=====================================
modules/control/dbus/dbus.c
=====================================
@@ -750,7 +750,6 @@ static void ProcessEvents( intf_thread_t *p_intf,
default:
vlc_assert_unreachable();
}
- free( p_events[i] );
}
if( !vlc_dictionary_is_empty( &player_properties ) )
@@ -762,6 +761,23 @@ static void ProcessEvents( intf_thread_t *p_intf,
if( !vlc_dictionary_is_empty( &root_properties ) )
RootPropertiesChangedEmit( p_intf, &root_properties );
+
+ for (int i = 0; i < i_events; i++)
+ {
+ switch (p_events[i]->signal)
+ {
+ case SIGNAL_PLAYLIST_ITEM_APPEND:
+ tracklist_append_event_destroy(p_events[i]->items_appended);
+ break;
+ case SIGNAL_PLAYLIST_ITEM_DELETED:
+ tracklist_remove_event_destroy(p_events[i]->items_removed);
+ break;
+ default:
+ break;
+ }
+ free(p_events[i]);
+ }
+
vlc_dictionary_clear( &player_properties, NULL, NULL );
vlc_dictionary_clear( &tracklist_properties, NULL, NULL );
vlc_dictionary_clear( &root_properties, NULL, NULL );
@@ -1030,8 +1046,18 @@ static bool add_event_locked( intf_thread_t *p_intf, const callback_info_t *p_in
{
callback_info_t *oldinfo =
vlc_array_item_at_index( &p_intf->p_sys->events, i );
- if( p_info->signal == oldinfo->signal )
- return false;
+
+ switch (p_info->signal)
+ {
+ /* Those events are squashed afterwards and must be
+ * released appropriately. */
+ case SIGNAL_PLAYLIST_ITEM_APPEND:
+ case SIGNAL_PLAYLIST_ITEM_DELETED:
+ break;
+ default:
+ if (p_info->signal == oldinfo->signal)
+ return false;
+ }
}
callback_info_t *p_dup = malloc( sizeof( *p_dup ) );
@@ -1061,9 +1087,11 @@ playlist_on_items_added(vlc_playlist_t *playlist, size_t index,
void *data)
{
tracklist_append_event_t *append_event = tracklist_append_event_create(index, items, count);
- add_event_signal(data,
+ bool added = add_event_signal(data,
&(callback_info_t){ .signal = SIGNAL_PLAYLIST_ITEM_APPEND,
.items_appended = append_event });
+ if (!added)
+ tracklist_append_event_destroy(append_event);
(void) playlist;
}
@@ -1072,9 +1100,11 @@ playlist_on_items_removed(vlc_playlist_t *playlist,
size_t index, size_t count, void *data)
{
tracklist_remove_event_t *remove_event = tracklist_remove_event_create(index, count);
- add_event_signal(data,
+ bool added = add_event_signal(data,
&(callback_info_t){ .signal = SIGNAL_PLAYLIST_ITEM_DELETED,
.items_removed = remove_event });
+ if (!added)
+ tracklist_remove_event_destroy(remove_event);
(void) playlist;
}
=====================================
modules/control/dbus/dbus_tracklist.c
=====================================
@@ -620,7 +620,6 @@ int TrackListPropertiesChangedEmit( intf_thread_t * p_intf,
added_tracks->items[i] );
}
tracklist_append_event_t *next = tracklist_append_event_next( added_tracks );
- tracklist_append_event_destroy( added_tracks );
added_tracks = next;
}
}
@@ -634,7 +633,6 @@ int TrackListPropertiesChangedEmit( intf_thread_t * p_intf,
TrackRemovedSignal( p_intf, removed_tracks->change_ev.index + i );
}
tracklist_remove_event_t *next = tracklist_remove_event_next( removed_tracks );
- tracklist_remove_event_destroy( removed_tracks );
removed_tracks = next;
}
=====================================
modules/control/dbus/dbus_tracklist.h
=====================================
@@ -74,7 +74,7 @@ void tracklist_append_event_destroy( tracklist_append_event_t *event );
void tracklist_remove_event_destroy( tracklist_remove_event_t *event );
/* Gets next event in the list */
-static tracklist_append_event_t *
+static inline tracklist_append_event_t *
tracklist_append_event_next( tracklist_append_event_t *event ) {
if( !event )
return NULL;
@@ -83,7 +83,7 @@ tracklist_append_event_next( tracklist_append_event_t *event ) {
(p - offsetof(struct tracklist_append_event, change_ev));
}
-static tracklist_remove_event_t *
+static inline tracklist_remove_event_t *
tracklist_remove_event_next( tracklist_remove_event_t *event ) {
if( !event )
return NULL;
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/17ada6c55fea1289c60475872b9613c4abb173fe...a168eb015bf51517ba5ad34b6ebe65c1c0cea2ef
--
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/17ada6c55fea1289c60475872b9613c4abb173fe...a168eb015bf51517ba5ad34b6ebe65c1c0cea2ef
You're receiving this email because of your account on code.videolan.org.
VideoLAN code repository instance
More information about the vlc-commits
mailing list