Mir

Code review comment for lp:~mir-team/mir/fix-1339700

Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

> > Alberto: framedrop_policy is a member of BufferQueue and as such is
> protected
> > by BufferQueue.guard.
>
> const members do not need to be protected by a lock.
>
> While framedrop_policy isn't declared const it is only set during constructor
> so it is an "honorary const" member.
>
> This isn't clear and I'd prefer a rewrite that makes framedrop_policy const
> but this does seem safe and fix the deadlock.

std::unique_ptr<FrameDroppingPolicy> is effectively const but the FrameDroppingPolicy object itself is not, so we need to ensure it's thread-safe. It's not obvious that our TimeoutFrameDroppingPolicy implementation is thread-safe; calling Alarm methods muddies the water.

review: Needs Information

« Back to merge proposal