[vlc-devel] [PATCH 01/12] input item: expose locked version of GetMeta()
rom1v at videolabs.io
Fri Oct 12 23:46:35 CEST 2018
On Fri, Oct 12, 2018 at 10:44:18PM +0300, Rémi Denis-Courmont wrote:
> Le perjantaina 12. lokakuuta 2018, 0.14.39 EEST Romain Vimont a écrit :
> > In order to sort the playlist correctly, the input items must be locked
> > across several calls to GetMeta() (among others).
> Taking multiple locks from the same class simultaneously is a terrible idea
> that will defeat lock dependency checkers (no that we use any, but we probably
I agree. I wrote it that way (patch 7) because I wanted all these
- _Sort() returning void (which implies sorting in place without memory
- not unnecessarily inefficient (no lock/unlock on every compare());
- a correct sorting (the items must not be updated during sorting).
> And it is theoretically only possible if there is an order, which this patch
> suspiciously fails to define.
The input items instances are not supposed to be used in several
playlists or services discoveries simultaneously (we copy them in that
case), and in practice, _Sort() should be the only function which may
lock several items, so lock-order inversion should never happen.
But yes, in theory, the clients may do what they want with input items,
including locking several of them, making lock-order inversion possible.
So it's quite fragile.
The alternative I have in mind is:
1. make _Sort() return a status (int) to report allocation failure;
2. create a separate temporary vector of a custom struct, containing
the useful properties of input_item_t and a reference to the
3. for each item, copy (with lock held) the properties which are
relevant for the requested sorting;
4. sort that temporary vector;
5. report the resulting order to the playlist items vector.
What do you think?
More information about the vlc-devel