Strange behaviour in Spec

Le 04/02/2013 17:27, Andrei Vasile Chis a écrit :

Hi,

While playing with spec I started to have some quite random and
strange behaviour.
I'll try to explain what happen, because it might be a nasty/not that
nasty bug.

I had a ListComposableModel and some very simple code like:
     aListComposableModel whenListChanged: [ :aList |
aListComposableModel setSelectedItem: aList first ].

 From time to time this code seem to randomly fail because the selection
wasn't set.
I manage to reduce the error to the following equivalent code:
    aListComposableModel whenListChanged: [ :aList |
aList = aListComposableModel listItems ].

What happen was that sometimes the list held by the model was the
previous list and not the new list,
which was actually correctly passes to the block.

This seem to be caused by the way ListComposableModel uses announcers to
update its internal state.
In ListComposableModel>>registerEvents there is the code:  listHolder
whenChangedDo: [ self refreshListItems ].

This registers an announcement that should clear the cache:
ListComposableModel >>refreshListItems
listItemsCache := nil.
^ self changed: #listElementAt:

In my code by using 'aListComposableModel whenListChanged:' I also
register an event that should be executed
when the elements of the list change.

So by now the listHolder containing the elements of the list has two
registered events.
The problem is that in order for this to work correctly they have to
execute in the order they
were added: first invalidate the cache, then the other events registered
by the user.
However I do not think that announcers guarantee this. So from time to
time they
execute in the wrong way. This means that the code in the
whenListChanged: block will
execute in a state in which internaly aListComposableModel still
contains the previous list.

Does this make sense?

Yes. It would mean the two announcers are kept in a Set... Maybe having private and public events would be the way to go ? A private one for clearing the cache and public ones, triggered after, for the whenListChanged: block ?

It would be nice to have a test case checking that. I'd be interested to know this can happen with announcements.

Thierry
--
Thierry Goubier
CEA list
Laboratoire des Fondations des Systèmes Temps Réel Embarqués
91191 Gif sur Yvette Cedex
France
Phone/Fax: +33 (0) 1 69 08 32 92 / 83 95

Hi Andrei,

I tested with the following code which shows some interesting effects:

|list|  
list := ListComposableModel new.

list whenListChanged: [:aList|
Transcript crShow: aList = list listItems].

10 timesRepeat: [
        list items: #(1 2).

        list listItems. "Call listItems to set the cache"

        list items: #(1 2 3).
        list listItems].

And I either get true-true-true-true or, true-false-false-false ...

When I get true-true-true, in list listHolder announcer registry subscriptions, I have the action

        [:aList| Transcript crShow: aList = list listItems]

set after the action
        [self refreshListItems].

When I get true-false-false-false ad nauseam, in list listHolder announcer registry subscriptions, I have the action

        [:aList| Transcript crShow: aList = list listItems]

set "before" the action
        [self refreshListItems].

So, it seems that the use of an identitySet in subscriptions for an announcer has interesting side effects.

So, what should be done ?

1- Change subscription in Announcer SubscriptionRegistry from IdentitySet to OrderedCollection : it should keep the same behavior in all users of Announcers, since they have to be order independent (otherwise, they would fall for the same intermittent bug given the chance).

2- Change ListComposableModel to use a private event to update the cache before entertaining a #changedValue event.

I have a preference for 1-, because of my soft spot for programs that have a nice, predictable behavior :)

I've added a ticket

http://code.google.com/p/pharo/issues/detail?id=7420

Thierry

Le 05/02/2013 11:31, Andrei Vasile Chis a écrit :

Hi,

A very simple way to reproduce is to open a transcript and execute this
code:
50 timesRepeat: [|list|
list := ListComposableModel new.
list items: #(1 2).
list whenListChanged: [:aList|
Transcript crShow: aList = list listItems].
list listItems. "Call listItems to set the cache"
list items: #(1 2 3) ].

If should print both true and false.
Every time it prins false the events were triggered in the wrong order.

Andrei

On Tue, Feb 5, 2013 at 9:26 AM, Goubier Thierry <[hidden email]

<mailto:[hidden email]>> wrote:

    Le 04/02/2013 17:27, Andrei Vasile Chis a écrit :

        Hi,

        While playing with spec I started to have some quite random and
        strange behaviour.
        I'll try to explain what happen, because it might be a nasty/not
        that
        nasty bug.

        I had a ListComposableModel and some very simple code like:
              aListComposableModel whenListChanged: [ :aList |
        aListComposableModel setSelectedItem: aList first ].

          From time to time this code seem to randomly fail because the
        selection
        wasn't set.
        I manage to reduce the error to the following equivalent code:
             aListComposableModel whenListChanged: [ :aList |
        aList = aListComposableModel listItems ].

        What happen was that sometimes the list held by the model was the
        previous list and not the new list,
        which was actually correctly passes to the block.

        This seem to be caused by the way ListComposableModel uses
        announcers to
        update its internal state.

        In ListComposableModel>>__registerEvents there is the code:

          listHolder
        whenChangedDo: [ self refreshListItems ].

        This registers an announcement that should clear the cache:
        ListComposableModel >>refreshListItems
        listItemsCache := nil.
        ^ self changed: #listElementAt:

        In my code by using 'aListComposableModel whenListChanged:' I also
        register an event that should be executed
        when the elements of the list change.

        So by now the listHolder containing the elements of the list has two
        registered events.
        The problem is that in order for this to work correctly they have to
        execute in the order they
        were added: first invalidate the cache, then the other events
        registered
        by the user.
        However I do not think that announcers guarantee this. So from
        time to
        time they
        execute in the wrong way. This means that the code in the
        whenListChanged: block will
        execute in a state in which internaly aListComposableModel still
        contains the previous list.

        Does this make sense?


    Yes. It would mean the two announcers are kept in a Set... Maybe
    having private and public events would be the way to go ? A private
    one for clearing the cache and public ones, triggered after, for the
    whenListChanged: block ?

    It would be nice to have a test case checking that. I'd be
    interested to know this can happen with announcements.

    Thierry
    --
    Thierry Goubier
    CEA list
    Laboratoire des Fondations des Systèmes Temps Réel Embarqués
    91191 Gif sur Yvette Cedex
    France
    Phone/Fax: +33 (0) 1 69 08 32 92 / 83 95

--
Thierry Goubier
CEA list
Laboratoire des Fondations des Systèmes Temps Réel Embarqués
91191 Gif sur Yvette Cedex
France
Phone/Fax: +33 (0) 1 69 08 32 92 / 83 95