<div dir="ltr">As Alan has pointed out the extra levels of indirection in an Observer implemention does makes lifetime an issue<div>or rather it requires that observer removal should guarantee the observer will not receive any more callbacks.</div> <div><br></div><div>It definitely needs more pondering time...<br><div><div><br></div><div><br><div><div><br></div><div><div><br></div></div></div></div></div></div></div><div class="gmail_extra"><br><br><div class="gmail_quote"> On Fri, Jun 27, 2014 at 10:07 AM, Alberto Aguirre <span dir="ltr"><<a href="mailto:alberto.aguirre@canonical.com" target="_blank">alberto.aguirre@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div dir="ltr">The problem with a recursive lock is that it may still lead to <div>subtle deadlocks if the observer generates new notifications on a different</div><div>thread - which may not be readily apparent to the person implementing the observer ,</div> <div>either because of complicated flow of events, or an external library may be called which indirectly causes a new notification. </div><div><br></div><div>Note that for SurfaceObserver and Scene observer specifically, the observers are stored in shared_ptr's so calling dead objects is not an issue there.</div> <div>But receiving at least an extra callback after registration is - which in this case, I would say it's the burden of the observer.</div><div><br></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"> <br><br><div class="gmail_quote"> On Fri, Jun 27, 2014 at 6:30 AM, Alan Griffiths <span dir="ltr"><<a href="mailto:alan.griffiths@canonical.com" target="_blank">alan.griffiths@canonical.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> We've had a number of proposals recently that ran into issues with when<br> and what to lock in collections of observers and what actions on the<br> observed object (and its collection of observers) are possible while in<br> a notification callback. (Some links at bottom)<br> <br> I don't know of a good approach that addresses all concerns (and a quick<br> internet search didn't find a good answer) but I thought I'd first write<br> out a list of the concerns that have crawled out of our discussions.<br> And mention what we've discovered so far.<br> <br> We've generally taken an approach that implies that the "observer"<br> objects making observations generally outlive the objects being<br> observed. Indeed the simplest case is that the observer is part of the<br> application infrastructure and not part of the dynamic state. For this<br> case it is simplest to create a "listener" object (that calls<br> notification methods on the observer) and pass its ownership to the<br> "subject". The observer can then forget about everything except handling<br> the notifications.<br> <br> The naive approach is for the collection of listeners to be locked when<br> being updated and when sending notifications.<br> <br> This works well until we hit one of two cases:<br> 1. the observer takes some action that generates a new notification,<br> 2. the observer is destroyed and needs to prevent further notifications<br> to a dead object.<br> <br> In case 2 we expect the observer to remove the listener from the<br> subject's collection *after* which notifications cease. Note that<br> "after" implies we need a synchronization mechanism as multiple threads<br> can be involved.<br> <br> In case 1, if we have hold exclusive lock during notifications then<br> we'll get a deadlock.<br> <br> One solution that has been tried is to copy the collection and release<br> the lock. That doesn't work with the above solution to case 2: after<br> releasing the lock nothing prevents listeners being "removed" on another<br> thread *before* being invoked through the copy.<br> <br> Another solution that has been tried is to hold a recursive lock during<br> notifications and updates to the collection. That allows other<br> notifications to take place *and changes to the collection*. So we still<br> take a copy of the collection and traverse that (to avoid iterators<br> invalidating), but before invoking them also check that objects exist in<br> the "true" collection. (There's an example in<br> MirEventDistributor::handle_event()).<br> <br> Thank you for listening!<br> <br> Alan<br> <br> <a href="https://code.launchpad.net/~albaguirre/mir/avoid-surf-observer-deadlocks/+merge/224733/comments/539885" target="_blank">https://code.launchpad.net/~albaguirre/mir/avoid-surf-observer-deadlocks/+merge/224733/comments/539885</a><br> <a href="https://code.launchpad.net/~andreas-pokorny/mir/synchronous-cancel-of-alarms" target="_blank">https://code.launchpad.net/~andreas-pokorny/mir/synchronous-cancel-of-alarms</a><br> <a href="https://code.launchpad.net/~alan-griffiths/mir/fix-1334287/+merge/224457/comments/538936" target="_blank">https://code.launchpad.net/~alan-griffiths/mir/fix-1334287/+merge/224457/comments/538936</a><br> <a href="https://code.launchpad.net/~mir-team/mir/trusted_sessions/+merge/221191/comments/532580" target="_blank">https://code.launchpad.net/~mir-team/mir/trusted_sessions/+merge/221191/comments/532580</a><br> <a href="https://code.launchpad.net/~mir-team/mir/introduce-scene-observer/+merge/216957" target="_blank">https://code.launchpad.net/~mir-team/mir/introduce-scene-observer/+merge/216957</a><br> <span><font color="#888888"><br> --<br> Mir-devel mailing list<br> <a href="mailto:Mir-devel@lists.ubuntu.com" target="_blank">Mir-devel@lists.ubuntu.com</a><br> Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/mir-devel" target="_blank">https://lists.ubuntu.com/mailman/listinfo/mir-devel</a><br> </font></span></blockquote></div><br></div> </div></div></blockquote></div><br></div>