The definition of read() mandates from its prototype that the return
value and errno must be checked since the syscall can be interrupted
without being processed by a signal:
../../modules/control/dbus/dbus.c: In function ‘Run’:
../../modules/control/dbus/dbus.c:958:19: warning: ignoring return value of ‘read’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
958 | (void)read( fds[0].fd, &buf, 1 );
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
In practice, the read is not done on a slow device, but a UNIX pipe
instead, and only when poll() notify that a read operation will be
non-blocking, and we're not supposed to have a signal handler here in
the normal execution (not using tools making use of SIGPROF for
instance), so read() being interrupted is nearly impossible, as opposed
to poll() being interrupted.
The read() call is also changed to use an anonymous compound litteral
which doesn't need to be marked as used manually, since we didn't use
the buffer variable at all.
In the future, using eventfd() to generate a semaphore compatible with
poll() and making the event handling lock-free might be a better
alternative, since read() will return the number of new events, and the
lock would become redundant.
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.
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.
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 #27780Fixes#28307
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.
Fixes a memory leak where elements in the linked list of events where
never released, due to the release function being always called at the
end of the traversal instead of every iteration.
Implements the following signals for the
org.mpris.MediaPlayer2.TrackList interface:
- TrackAdded: a track was inserted to the list
- TrackRemoved: a track was removed from the list
The track index is sometimes known when serialising a track's metadata.
This moves the index query outside the metadata serialisation function
when necessary.
Snapshots are performed via a call to:
var_TriggerCallback("video-snapshot")
causing its callback (SnapshotCallback) to be called synchronously,
executing the following steps:
- wait for the vout thread to actually capture the (next) frame;
- encode the picture to PNG;
- write the result to disk (I/O).
Since var_TriggerCallback("video-snapshot") is called from the UI
thread, all these blocking actions are also performed on the UI thread.
Move the call to a separate thread.
It is not used in POSIX systems. On other system it probably don't make a
difference anymore, only Windows has actual useful values for
VLC_THREAD_PRIORITY_XXX. The synchronization is more important than having some
threads called more often than others.
see e967f81f6a.
note, this does **not** affect cat-based module selection items
(of which there are just three in use by the core), since that
mechanism uses subcats not cats.
Alexandre Janniaux