commit c2b6af394e0e9ee511e9e44a54ddc6e40ef0e0bf
author Daniel Almeida <daniel.almeida@collabora.com> Fri May 26 16:47:51 2023 -0300
committer Alexandre Courbot <gnurou@gmail.com> Thu Jun 08 12:27:35 2023 +0900
tree 3abe5147bb1c43def9c8fc0bdfe2ddbd593404b2
parent 54913a51fbb3fbf9114a54b201745cb51d6c91ed

utils: vaapi: rework the Rc and Refcell in SurfacePoolHandle

This type has a bug that has gone unnoticed for quite a while.

In order to get better ergonomics and hide away the
borrow()/borrow_mut() calls, the Rc<RefCell<T>> was placed in the
"surfaces" member. #derive(Clone) was also added so that this type could
be cloned without the hassle of the Rc::clone(..) syntax.

This means that calling SurfacePoolHandle::clone() will clone the inner
Rc, i.e. all handles will automatically refer to the same VecDeque. It
will also clone the "coded_resolution" member, which will obviously just
bit-wise copy the u32.

This means that all invariants based on the "coded_resolution" member
will be silently rendered useless, as each SurfacePoolHandle instance
will refer to its own copy of "coded_resolution" to enforce them.

This is not what this type was intended for. It only went unnoticed
because a new pool would be created on every DRC, and the
'coded_resolution' member would never be updated on the old pool.

To fix this, move the Rc<RefCell<T>> to the outer type, such that we can
refer to a single instance for all members of the pool struct. Also,
rename this type from SurfacePoolHandle to SurfacePool, to indicate that
users must heap-allocate the struct themselves if they want to share it.

This fix will let us reuse the pool on DRC without breaking its
invariants.