Observers, collections and multiple threads

[Alberto Aguirre]

As Alan has pointed out the extra levels of indirection in an Observer
implemention does makes lifetime an issue
or rather it requires that observer removal should guarantee the observer
will not receive any more callbacks.

It definitely needs more pondering time...






On Fri, Jun 27, 2014 at 10:07 AM, Alberto Aguirre <
alberto.aguirre at canonical.com> wrote:

> The problem with a recursive lock is that it may still lead to
> subtle deadlocks if the observer generates new notifications on a different
> thread - which may not be readily apparent to the person implementing the
> observer ,
> either because of complicated flow of events, or an external library may
> be called which indirectly causes a new notification.
>
> 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.
> But receiving at least an extra callback after registration is - which in
> this case, I would say it's the burden of the observer.
>
>
>
> On Fri, Jun 27, 2014 at 6:30 AM, Alan Griffiths <
> alan.griffiths at canonical.com> wrote:
>
>> We've had a number of proposals recently that ran into issues with when
>> and what to lock in collections of observers and what actions on the
>> observed object (and its collection of observers) are possible while in
>> a notification callback. (Some links at bottom)
>>
>> I don't know of a good approach that addresses all concerns (and a quick
>> internet search didn't find a good answer) but I thought I'd first write
>> out a list of the  concerns that have crawled out of our discussions.
>> And mention what we've discovered so far.
>>
>> We've generally taken an approach that implies that the "observer"
>> objects making observations generally outlive the objects being
>> observed. Indeed the simplest case is that the observer is part of the
>> application infrastructure and not part of the dynamic state. For this
>> case it is simplest to create a "listener" object (that calls
>> notification methods on the observer) and pass its ownership to the
>> "subject". The observer can then forget about everything except handling
>> the notifications.
>>
>> The naive approach is for the collection of listeners to be locked when
>> being updated and when sending notifications.
>>
>> This works well until we hit one of two cases:
>> 1. the observer takes some action that generates a new notification,
>> 2. the observer is destroyed and needs to prevent further notifications
>> to a dead object.
>>
>> In case 2 we expect the observer to remove the listener from the
>> subject's collection *after* which notifications cease. Note that
>> "after" implies we need a synchronization mechanism as multiple threads
>> can be involved.
>>
>> In case 1, if we have hold exclusive lock during notifications then
>> we'll get a deadlock.
>>
>> One solution that has been tried is to copy the collection and release
>> the lock. That doesn't work with the above solution to case 2: after
>> releasing the lock nothing prevents listeners being "removed" on another
>> thread *before* being invoked through the copy.
>>
>> Another solution that has been tried is to hold a recursive lock during
>> notifications and updates to the collection. That allows other
>> notifications to take place *and changes to the collection*. So we still
>> take a copy of the collection and traverse that (to avoid iterators
>> invalidating), but before invoking them also check that objects exist in
>> the "true" collection. (There's an example in
>> MirEventDistributor::handle_event()).
>>
>> Thank you for listening!
>>
>> Alan
>>
>>
>> https://code.launchpad.net/~albaguirre/mir/avoid-surf-observer-deadlocks/+merge/224733/comments/539885
>>
>> https://code.launchpad.net/~andreas-pokorny/mir/synchronous-cancel-of-alarms
>>
>> https://code.launchpad.net/~alan-griffiths/mir/fix-1334287/+merge/224457/comments/538936
>>
>> https://code.launchpad.net/~mir-team/mir/trusted_sessions/+merge/221191/comments/532580
>>
>> https://code.launchpad.net/~mir-team/mir/introduce-scene-observer/+merge/216957
>>
>> --
>> Mir-devel mailing list
>> Mir-devel at lists.ubuntu.com
>> Modify settings or unsubscribe at:
>> https://lists.ubuntu.com/mailman/listinfo/mir-devel
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/mir-devel/attachments/20140627/566dac1a/attachment-0001.html>