Mir

Code review comment for lp:~robertcarr/mir/client-focus-notifications

  1. client-focus-notifications
  2. Merge into trunk
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

CloggableEventSink looks horribly broken:

There is no synchronization around "216 + if (clogged)" - which means it could be in different states in different threads.

"clogged" could change between "if (clogged)" being evaluated and the next statement.

There is nothing to prevent multiple calls to clog() - perhaps clogged should be an atomic counter?

I assume that clog() and unclog() are intended to be paired operations - but the calls I see don't deal with exceptions being thrown (please use RAII for paired operations).

What is the motivation for CloggableEventSink? Is this the right place for synchronization?

review: Needs Fixing

« Back to merge proposal