Race condition(s) in Announcement framework

>---- Original message ----
>>Date: Fri, 12 May 2006 10:28:00 +0200
>>From: Ladislav Lenart <[hidden email]>  
>>Subject: Race condition(s) in Announcement framework  
>>To: vwnc <[hidden email]>
>>
>>Hi,
>>
>>We have found two problems in Announcement framework when
>using weak announcements.
>>
>>Both problems are closely coupled with fact that
>WeakAnnouncementSubscription removes itself
>>from registry in its #mourn method:
>>
>>[1]
>>I think that it should not be considered an error when the
>registry does not contain
>>the subscription anymore. This can easily happen when the
>subscriber sends #unsubscribe:.
>>So I would recommend using something like:
>> WeakAnnouncementSubscription>>mourn
>> subscriber := nil.
>> destination := nil.
>> registry safelyRemoveSubscription: self.
>> "like registry removeSubscription: self ifAbsent:[]"
>>
>>[2]
>>The more serious problem is that a subscription can receive
>#mourn *while* its registry
>>is delivering an announcement. This leads to iterating over
>collection which is being
>>modified. There is no easy fix like adding #copy to
>SubscriptionRegistry>>defliver:from:
>>because the subscription is no longer usable after it is

> The more serious problem is that a subscription can receive #mourn
> *while* its registry
> is delivering an announcement. This leads to iterating over collection
> which is being
> modified. There is no easy fix like adding #copy to
> SubscriptionRegistry>>defliver:from:
> because the subscription is no longer usable after it is mourned.
>
> Any ideas how to solve the second problem?
>
> Ladislav Lenart

> I'm using a combo box with aspect paths for buffering, and using the
> Print and Read properties.
>
> Now once I install the read and print methods the Canvas seems not to
> send any #change message and the updateTrigger method will not be fired.
>
> As consequence, the model does not get updated?
>
> I made a short example using a boolean variable in a simple model and
> when the Type property in the combo box is Boolean, the protocol works.
>
> Once I change the Type to Object and put my read and print methods in
> Read and Print properties, the list doesn't work anymore.
>
> Is there a means to put in the read an print methods some sort of self
> changed: to something (as I have only the aspect path)?
>

> [hidden email] wrote:
>> I am not familiar with Announcement but it sounds like it
>> needs to have a critical section around the piece of code that
>> delivers the events to the subscribers.
>
> Well, that is one possible solution.
>
> Another would be to set some flag like #mourned in
> WeakAnnouncementSubscription>>mourn instead of directly removing
> itself from the subscription registry. When the registry is delivering
> an announcement, it can purge such dead
> subscriptions. And because the subscriptions are already mourned, they
> do not prevent anything from being gc'ed,
> only the subscriptions themselves will live a little longer which is not
> a big deal.
>
>> Do you have a piece of code we can test? we are presenting a
>> paper in September about data races and frameworks in COMPSAC.
>> This could be another good example to use our tools.
>
> Well, it was a bit tricky but I have it (see attachment).
>
> In order to run it, you'll need the following packages from public
> repository:
>     1) System-Announcement package
>     2) SUnitToo package
>     3) ExtraRBForSUnitToo package (optional CodeTool for RB)
>
> Once you have them in your image, you can fileIn my attachment, which
> contains:
>     1) Announcer - object that has *weak* subscriptions and announces
> SomethingHappened announcement.
>     2) Subscriber - object listening on its Announcer for
> SomethingHappened.
>     3) SomethingHappened - passed announcement.
>     4) RaceConditionTest - TestCase with one test method:
> #test_raceCondition.
> Everything is in Smalltalk environment in package called
> "_runtime_additions_".
>
> Run it and you should get DNU, but don't be surprised that it is *after*
> the test already passed,
> because the test itself only starts two forked blocks.
>
> I also had to add printing to Transcript because otherwise it was all
> too quick and nothing bad happened... :-)
>
> I would also like to note that in our original case we do not use more
> threads - it's just that the structure
> is complicated enough that it takes some time till it is gc'ed. So one
> process is ours and other one is the GC
> itself.
>
> Hope this helps,
>
> Ladislav Lenart
>
>> Federico
>>
>>
>>> ---- Original message ----
>>>
>>>> Date: Fri, 12 May 2006 10:28:00 +0200
>>>> From: Ladislav Lenart <[hidden email]>  Subject: Race
>>>> condition(s) in Announcement framework  To: vwnc <[hidden email]>
>>>>
>>>> Hi,
>>>>
>>>> We have found two problems in Announcement framework when
>>>
>>> using weak announcements.
>>>
>>>> Both problems are closely coupled with fact that
>>>
>>> WeakAnnouncementSubscription removes itself
>>>> from registry in its #mourn method:
>>>
>>>> [1]
>>>> I think that it should not be considered an error when the
>>>
>>> registry does not contain
>>>
>>>> the subscription anymore. This can easily happen when the
>>>
>>> subscriber sends #unsubscribe:.
>>>
>>>> So I would recommend using something like:
>>>>     WeakAnnouncementSubscription>>mourn
>>>>         subscriber := nil.
>>>>         destination := nil.
>>>>         registry safelyRemoveSubscription: self.
>>>>             "like registry removeSubscription: self ifAbsent:[]"
>>>>
>>>> [2]
>>>> The more serious problem is that a subscription can receive
>>>
>>> #mourn *while* its registry
>>>
>>>> is delivering an announcement. This leads to iterating over
>>>
>>> collection which is being
>>>
>>>> modified. There is no easy fix like adding #copy to
>>>
>>> SubscriptionRegistry>>defliver:from:
>>>
>>>> because the subscription is no longer usable after it is
>>
>> mourned.
>>
>>>> Any ideas how to solve the second problem?
>>>>
>>>> Ladislav Lenart
>>>>
>
> ------------------------------------------------------------------------
>
> <?xml version="1.0"?>
>
> <st-source>
> <time-stamp>From VisualWorks®, 7.4 of 5. prosinec 2005 on 12. květen 2006 at 18:08:56</time-stamp>
>
>
> <class>
> <name>Announcer</name>
> <environment>Smalltalk</environment>
> <super>Core.Object</super>
> <private>false</private>
> <indexed-type>none</indexed-type>
> <inst-vars></inst-vars>
> <class-inst-vars></class-inst-vars>
> <imports></imports>
> <category>My Classes</category>
> <attributes>
> <package>_runtime_additions_</package>
> </attributes>
> </class>
>
> <methods>
> <class-id>Announcer</class-id> <category>actions</category>
>
> <body package="_runtime_additions_" selector="announce">announce
>
> self announce: SomethingHappened.</body>
> </methods>
>
> <methods>
> <class-id>Announcer</class-id> <category>private</category>
>
> <body package="_runtime_additions_" selector="createSubscriptionRegistry">createSubscriptionRegistry
>
> ^SubscriptionRegistry new subscriptionClass: WeakAnnouncementSubscription.</body>
> </methods>
>
> <class>
> <name>RaceConditionTest</name>
> <environment>Smalltalk</environment>
> <super>SUnit.TestCase</super>
> <private>false</private>
> <indexed-type>none</indexed-type>
> <inst-vars>announcer subscribers </inst-vars>
> <class-inst-vars></class-inst-vars>
> <imports></imports>
> <category>My Classes</category>
> <attributes>
> <package>_runtime_additions_</package>
> </attributes>
> </class>
>
> <methods>
> <class-id>RaceConditionTest</class-id> <category>tests</category>
>
> <body package="_runtime_additions_" selector="test_raceCondition">test_raceCondition
>
> | block1 block2 |
> block1 := [
> (1 to: 100) do: [:ignored |
> Transcript show: 'Removing last. '.
> subscribers removeLast.
> Processor yield.
> ].
> ].
> block2 := [
> announcer announce.
> ].
> block1 forkAt: Processor activePriority -1.
> block2 forkAt: Processor activePriority -1.</body>
> </methods>
>
> <methods>
> <class-id>RaceConditionTest</class-id> <category>initialize-release</category>
>
> <body package="_runtime_additions_" selector="setUp">setUp
>
> announcer := Announcer new.
> subscribers := ((1 to: 100) collect: [:ignored | Subscriber announcer: announcer]) asOrderedCollection.</body>
> </methods>
>
> <class>
> <name>SomethingHappened</name>
> <environment>Smalltalk</environment>
> <super>Core.Announcement</super>
> <private>false</private>
> <indexed-type>none</indexed-type>
> <inst-vars></inst-vars>
> <class-inst-vars></class-inst-vars>
> <imports></imports>
> <category>My Classes</category>
> <attributes>
> <package>_runtime_additions_</package>
> </attributes>
> </class>
>
> <class>
> <name>Subscriber</name>
> <environment>Smalltalk</environment>
> <super>Core.Object</super>
> <private>false</private>
> <indexed-type>none</indexed-type>
> <inst-vars>announcer </inst-vars>
> <class-inst-vars></class-inst-vars>
> <imports></imports>
> <category>My Classes</category>
> <attributes>
> <package>_runtime_additions_</package>
> </attributes>
> </class>
>
> <!-- -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -   -->
>
>
> <methods>
> <class-id>Subscriber class</class-id> <category>instance creation</category>
>
> <body package="_runtime_additions_" selector="announcer:">announcer: anAnnouncer
>
> ^self new setAnnouncer: anAnnouncer.</body>
> </methods>
>
> <!-- -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -  -   -->
>
>
> <methods>
> <class-id>Subscriber</class-id> <category>private</category>
>
> <body package="_runtime_additions_" selector="hookToAnnouncer">hookToAnnouncer
>
> announcer
> when: SomethingHappened
> send: #somethingHappened
> to: self.</body>
>
> <body package="_runtime_additions_" selector="setAnnouncer:">setAnnouncer: anAnnouncer
>
> announcer := anAnnouncer.
> self hookToAnnouncer.</body>
> </methods>
>
> <methods>
> <class-id>Subscriber</class-id> <category>event evaluating</category>
>
> <body package="_runtime_additions_" selector="somethingHappened">somethingHappened
>
> Transcript show: 'Something happened. '.
> Processor yield.</body>
> </methods>
>
> </st-source>

> Ladislav Lenart wrote:
>
>> The more serious problem is that a subscription can receive #mourn
>> *while* its registry
>> is delivering an announcement. This leads to iterating over collection
>> which is being
>> modified. There is no easy fix like adding #copy to
>> SubscriptionRegistry>>defliver:from:
>> because the subscription is no longer usable after it is mourned.
>>
>> Any ideas how to solve the second problem?
>>
>> Ladislav Lenart
>
>
> Is this a hypothetical problem or something you observed? Registry does
> copy-on-write when it adds and removes subscriptions, specifically to
> avoid iterating over a collection being modified.

> Vassili Bykov wrote:
>
>> Ladislav Lenart wrote:
>>
>>> The more serious problem is that a subscription can receive #mourn
>>> *while* its registry
>>> is delivering an announcement. This leads to iterating over
>>> collection which is being
>>> modified. There is no easy fix like adding #copy to
>>> SubscriptionRegistry>>defliver:from:
>>> because the subscription is no longer usable after it is mourned.
>>>
>>> Any ideas how to solve the second problem?
>>>
>>> Ladislav Lenart
>>
>>
>>
>> Is this a hypothetical problem or something you observed? Registry
>> does copy-on-write when it adds and removes subscriptions,
>> specifically to avoid iterating over a collection being modified.
>
>
> Well, it happens in our case but I just found out that there is new
> (friday)
> version of Announcements in the public repository that seems to address
> this
> issue, so I'll give it a try and let you know.