Mir

Code review comment for lp:~vanvugt/mir/simplify-single-object-accesses

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> The assertion that actions were previously under lock is incorrect. See:
>
> 21 - lk.unlock();
> 22 - exec(surface.get());
>
> So this proposal does not change the locking behaviour at all.

Yikes! The existing code is confusing: The *point* of execute-around is to run paired operations *around* the invoked code (e.g. start transaction/end transaction).

It is far too easy to make the mistake I did and assume the lock is being held. If we're not holding the lock then either /1/ there's no need for this pattern; or, /2/ releasing the lock before exec() is an existing error.

Either way, this MP doesn't break things.

review: Approve

« Back to merge proposal