[vlc-devel] [PATCH 2/4] libvlc: media_player: use vlc_atomic_rc for refcount

[Rémi Denis-Courmont]

Le lundi 23 novembre 2020, 15:25:19 EET Alexandre Janniaux a écrit :
> On Mon, Nov 23, 2020 at 11:13:04AM +0100, Romain Vimont wrote:
> > On Mon, Nov 23, 2020 at 11:13:55AM +0200, Rémi Denis-Courmont wrote:
> > > Hi,
> > > 
> > > I don't really the point in using an atomic variable if it can't avoid
> > > the
> > > mutex.
> > 
> > But the mutex locking is avoided for refcounting (and this mutex is for
> > other things).
> 
> In addition to the fact it's actually removing mutex in some calls,
> it's also removing a lot of confusing locking code, including potential
> future maintenance errors because of missing unlock on the non-release
> path. Atomic are also not subject to lock inversion issues or such, and
> even in the simplest case of my patch, it is at most replacing one
> variable by another one, so not really adding complexity here.
> 
> Moreover, having a common base for refcounting in every libvlc object
> helps readability a lot.

Replacing hand-coded atomics is a case of get-some-loose-some. It does save 
having to worry about memory ordering, and explicitly identifies a reference 
counter as such. But the boolean result value is unintuitive compared to 
checking the reference count.

In this case, there are no atomics to replace anyway, so you don't really get 
anything as such.

-- 
Rémi Denis-Courmont