Smalltalk › Squeak › Squeak - Dev

Example of non-concurrent code :)

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

10 messages Options

Igor Stasenko

Example of non-concurrent code :)

mouseTrailFrom: currentBuf
"Current event, a mouse event buffer, is about to be processed. If
there are other similar mouse events queued up, then drop them from
the queue, and report the positions inbetween."

| nextEvent trail |
trail := (Array new: 1) writeStream.
trail nextPut: currentBuf third @ currentBuf fourth.
*a* [(nextEvent := Sensor peekEvent) isNil] whileFalse:
[nextEvent first = currentBuf first
ifFalse: [^trail contents "different event type"].
nextEvent fifth = currentBuf fifth
ifFalse: [^trail contents "buttons changed"].
nextEvent sixth = currentBuf sixth
ifFalse: [^trail contents "modifiers changed"].
"nextEvent is similar. Remove it from the queue, and check the next."
*b* nextEvent := Sensor nextEvent.
trail nextPut: nextEvent third @ nextEvent fourth].
^trail contents

Here, the bug:
first it sends #peekEvent and its not nil.
Okay, then after some logic voodo, it sends nextEvent, without
precaution, that it may also answer nil!

So, if some other process get between these two sends (*a*, *b*) and
fetch/flush all events, a code will fail,
because
'nextEvent third' is DNU , when nextEvent == nil.

The fix is simple:

nextEvent := Sensor nextEvent.
nextEvent ifNotNil: [ trail nextPut: nextEvent third @ nextEvent fourth]

Both Pharo & Squeak contain this bug. But this method slightly differs
from each other.

--
Best regards,
Igor Stasenko AKA sig.

Andreas.Raab

Re: Example of non-concurrent code :)

On 10/15/2010 9:41 PM, Igor Stasenko wrote:
> Here, the bug:
> first it sends #peekEvent and its not nil.
> Okay, then after some logic voodo, it sends nextEvent, without
> precaution, that it may also answer nil!
>
> So, if some other process get between these two sends (*a*, *b*) and
> fetch/flush all events, a code will fail,

But there is no such code. The method in question is run from within
Morphic so unless there is a concurrent thread which for some reason
flushes the event queue[*1], this case simply cannot happen. The shared
queue used for synchronization here is specifically for synchronizing
the event queue with the Morphic event loop. It's not for general
purpose let's-just-dump-all-events-at-random-points-in-time usage.

[*1] I would claim that such code itself buggy. Flushing the event queue
should be synchronized with Morphic.

Cheers,
- Andreas

> The fix is simple:
>
> nextEvent := Sensor nextEvent.
> nextEvent ifNotNil: [ trail nextPut: nextEvent third @ nextEvent fourth]
>
> Both Pharo& Squeak contain this bug. But this method slightly differs
> from each other.
>

johnmci

Re: [Pharo-project] Example of non-concurrent code :)

In reply to this post by Igor Stasenko

Igor, take a look back in the messages from January 25th 2010
Re: [Pharo-project] new mac menu code
There may be something related there?

On 2010-10-15, at 9:41 PM, Igor Stasenko wrote:

> mouseTrailFrom: currentBuf
> "Current event, a mouse event buffer, is about to be processed. If
> there are other similar mouse events queued up, then drop them from
> the queue, and report the positions inbetween."
>
> | nextEvent trail |
> trail := (Array new: 1) writeStream.
> trail nextPut: currentBuf third @ currentBuf fourth.
> *a* [(nextEvent := Sensor peekEvent) isNil] whileFalse:
> [nextEvent first = currentBuf first
> ifFalse: [^trail contents "different event type"].
> nextEvent fifth = currentBuf fifth
> ifFalse: [^trail contents "buttons changed"].
> nextEvent sixth = currentBuf sixth
> ifFalse: [^trail contents "modifiers changed"].
> "nextEvent is similar. Remove it from the queue, and check the next."
> *b* nextEvent := Sensor nextEvent.
> trail nextPut: nextEvent third @ nextEvent fourth].
> ^trail contents
>
>
> Here, the bug:
> first it sends #peekEvent and its not nil.
> Okay, then after some logic voodo, it sends nextEvent, without
> precaution, that it may also answer nil!
>
> So, if some other process get between these two sends (*a*, *b*) and
> fetch/flush all events, a code will fail,
> because
> 'nextEvent third' is DNU , when nextEvent == nil.
>
> The fix is simple:
>
> nextEvent := Sensor nextEvent.
> nextEvent ifNotNil: [ trail nextPut: nextEvent third @ nextEvent fourth]
>
> Both Pharo & Squeak contain this bug. But this method slightly differs
> from each other.
>
> --
> Best regards,
> Igor Stasenko AKA sig.
>
> _______________________________________________
> Pharo-project mailing list
> [hidden email]
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project

--
===========================================================================
John M. McIntosh <[hidden email]> Twitter: squeaker68882
Corporate Smalltalk Consulting Ltd. http://www.smalltalkconsulting.com
===========================================================================

smime.p7s (5K) Download Attachment

Igor Stasenko

Re: Example of non-concurrent code :)

In reply to this post by Andreas.Raab

On 16 October 2010 08:00, Andreas Raab <[hidden email]> wrote:

> On 10/15/2010 9:41 PM, Igor Stasenko wrote:
>>
>> Here, the bug:
>> first it sends #peekEvent and its not nil.
>> Okay, then after some logic voodo, it sends nextEvent, without
>> precaution, that it may also answer nil!
>>
>> So, if some other process get between these two sends (*a*, *b*) and
>> fetch/flush all events, a code will fail,
>
> But there is no such code. The method in question is run from within Morphic
> so unless there is a concurrent thread which for some reason flushes the
> event queue[*1], this case simply cannot happen. The shared queue used for
> synchronization here is specifically for synchronizing the event queue with
> the Morphic event loop. It's not for general purpose
> let's-just-dump-all-events-at-random-points-in-time usage.
>
> [*1] I would claim that such code itself buggy. Flushing the event queue
> should be synchronized with Morphic.
>

There are 209 references to Sensor in my image.
How i can be sure that every place does not using it outside a Morphic
UI process?

> Cheers,
> - Andreas
>
>> The fix is simple:
>>
>> nextEvent := Sensor nextEvent.
>> nextEvent ifNotNil: [ trail nextPut: nextEvent third @ nextEvent fourth]
>>
>> Both Pharo& Squeak contain this bug. But this method slightly differs
>> from each other.
>>
>
>
>

--
Best regards,
Igor Stasenko AKA sig.

Igor Stasenko

Re: [Pharo-project] Example of non-concurrent code :)

In reply to this post by johnmci

On 16 October 2010 08:14, John M McIntosh
<[hidden email]> wrote:
> Igor, take a look back in the messages from January 25th 2010
> Re: [Pharo-project] new mac menu code
> There may be something related there?
>
>
No, its not related, because i'm not running it on Mac.

It is the bug in my own FIFOQueue implementation, which triggered the
bugs all over the places where following assumed:

queue isEmpty ifFalse: [
self assert: queue nextOrNil notNil.
]

or

queue peek ifNotNil: [
self assert: queue nextOrNil notNil.
]

neither #peek not #isEmpty does not guarantees that at next send , a
queue will contain any items.
Its actually completely pointless to do such checks.

One should use #next , for blocking version , which blocks the caller
if there's no items in queue,
or use nextOrNil, for quick non-blocking attempt to fetch item from queue.

--
Best regards,
Igor Stasenko AKA sig.

Andreas.Raab

Re: Example of non-concurrent code :)

In reply to this post by Igor Stasenko

On 10/15/2010 10:16 PM, Igor Stasenko wrote:

> On 16 October 2010 08:00, Andreas Raab<[hidden email]> wrote:
>> On 10/15/2010 9:41 PM, Igor Stasenko wrote:
>>>
>>> Here, the bug:
>>> first it sends #peekEvent and its not nil.
>>> Okay, then after some logic voodo, it sends nextEvent, without
>>> precaution, that it may also answer nil!
>>>
>>> So, if some other process get between these two sends (*a*, *b*) and
>>> fetch/flush all events, a code will fail,
>>
>> But there is no such code. The method in question is run from within Morphic
>> so unless there is a concurrent thread which for some reason flushes the
>> event queue[*1], this case simply cannot happen. The shared queue used for
>> synchronization here is specifically for synchronizing the event queue with
>> the Morphic event loop. It's not for general purpose
>> let's-just-dump-all-events-at-random-points-in-time usage.
>>
>> [*1] I would claim that such code itself buggy. Flushing the event queue
>> should be synchronized with Morphic.
>
> There are 209 references to Sensor in my image.
> How i can be sure that every place does not using it outside a Morphic
> UI process?

You can be just as sure as arguing that Set>>do: isn't thread safe. How
can you be sure that every place does not use it in a separate thread?
This kind of argument is pointless. If you have evidence that this is
indeed a problem, then let's fix it (rather: let's fix the place that
accesses UI stuff from another thread). But let's not pick a random
method in the image and go all wild about that it might perhaps not be
thread safe.

Cheers,
- Andreas

Igor Stasenko

Re: Example of non-concurrent code :)

On 16 October 2010 08:37, Andreas Raab <[hidden email]> wrote:

> On 10/15/2010 10:16 PM, Igor Stasenko wrote:
>>
>> On 16 October 2010 08:00, Andreas Raab<[hidden email]> wrote:
>>>
>>> On 10/15/2010 9:41 PM, Igor Stasenko wrote:
>>>>
>>>> Here, the bug:
>>>> first it sends #peekEvent and its not nil.
>>>> Okay, then after some logic voodo, it sends nextEvent, without
>>>> precaution, that it may also answer nil!
>>>>
>>>> So, if some other process get between these two sends (*a*, *b*) and
>>>> fetch/flush all events, a code will fail,
>>>
>>> But there is no such code. The method in question is run from within
>>> Morphic
>>> so unless there is a concurrent thread which for some reason flushes the
>>> event queue[*1], this case simply cannot happen. The shared queue used
>>> for
>>> synchronization here is specifically for synchronizing the event queue
>>> with
>>> the Morphic event loop. It's not for general purpose
>>> let's-just-dump-all-events-at-random-points-in-time usage.
>>>
>>> [*1] I would claim that such code itself buggy. Flushing the event queue
>>> should be synchronized with Morphic.
>>
>> There are 209 references to Sensor in my image.
>> How i can be sure that every place does not using it outside a Morphic
>> UI process?
>
> You can be just as sure as arguing that Set>>do: isn't thread safe. How can
> you be sure that every place does not use it in a separate thread? This kind
> of argument is pointless. If you have evidence that this is indeed a
> problem, then let's fix it (rather: let's fix the place that accesses UI
> stuff from another thread). But let's not pick a random method in the image
> and go all wild about that it might perhaps not be thread safe.
>

That's why topic starts from 'example' word.
For example, lets pick a random method in the image... like #mouseTrailFrom:

:)

> Cheers,
> - Andreas
>
>

--
Best regards,
Igor Stasenko AKA sig.

marcel.taeumel (old)

AW: [squeak-dev] Re: Example of non-concurrent code :)

In reply to this post by Andreas.Raab

So Set>>do: is not thread-safe. That's an interesting point. I think it would really improve the quality of the present API if all standard containers would be thread-safe. Is there already a project that tries to achieve this?

Ciao,
Marcel

-----Ursprüngliche Nachricht-----
Von: [hidden email] [mailto:[hidden email]] Im Auftrag von Andreas Raab
Gesendet: Samstag, 16. Oktober 2010 07:38
An: The general-purpose Squeak developers list
Betreff: [squeak-dev] Re: Example of non-concurrent code :)

On 10/15/2010 10:16 PM, Igor Stasenko wrote:

Igor Stasenko

Re: Example of non-concurrent code :)

In reply to this post by Andreas.Raab

On 16 October 2010 08:55, Marcel Taeumel
<[hidden email]> wrote:
> So Set>>do: is not thread-safe. That's an interesting point. I think it would really improve the quality of the present API if all standard containers would be thread-safe. Is there already a project that tries to achieve this?
>

We are discussed this before.
My own opinion on it: do not try to make standard collections like Set
thread safe,
because it will lead to fine-grained locking, which will make image
crawl like a snail.
This is wrong direction.

You should use either specialized containers, like SharedQueue,
or protect your data from concurrent access at more coarse-grained level.

> Ciao,
> Marcel
>
> -----Ursprüngliche Nachricht-----
> Von: [hidden email] [mailto:[hidden email]] Im Auftrag von Andreas Raab
> Gesendet: Samstag, 16. Oktober 2010 07:38
> An: The general-purpose Squeak developers list
> Betreff: [squeak-dev] Re: Example of non-concurrent code :)
>
> On 10/15/2010 10:16 PM, Igor Stasenko wrote:
>> On 16 October 2010 08:00, Andreas Raab<[hidden email]> wrote:
>>> On 10/15/2010 9:41 PM, Igor Stasenko wrote:
>>>>
>>>> Here, the bug:
>>>> first it sends #peekEvent and its not nil.
>>>> Okay, then after some logic voodo, it sends nextEvent, without
>>>> precaution, that it may also answer nil!
>>>>
>>>> So, if some other process get between these two sends (*a*, *b*) and
>>>> fetch/flush all events, a code will fail,
>>>
>>> But there is no such code. The method in question is run from within Morphic
>>> so unless there is a concurrent thread which for some reason flushes the
>>> event queue[*1], this case simply cannot happen. The shared queue used for
>>> synchronization here is specifically for synchronizing the event queue with
>>> the Morphic event loop. It's not for general purpose
>>> let's-just-dump-all-events-at-random-points-in-time usage.
>>>
>>> [*1] I would claim that such code itself buggy. Flushing the event queue
>>> should be synchronized with Morphic.
>>
>> There are 209 references to Sensor in my image.
>> How i can be sure that every place does not using it outside a Morphic
>> UI process?
>
> You can be just as sure as arguing that Set>>do: isn't thread safe. How
> can you be sure that every place does not use it in a separate thread?
> This kind of argument is pointless. If you have evidence that this is
> indeed a problem, then let's fix it (rather: let's fix the place that
> accesses UI stuff from another thread). But let's not pick a random
> method in the image and go all wild about that it might perhaps not be
> thread safe.
>
> Cheers,
> - Andreas
>
>
>
>

--
Best regards,
Igor Stasenko AKA sig.

marcel.taeumel (old)

AW: [squeak-dev] Re: Example of non-concurrent code :)

Where did you get this experience from? I do not know the impact in Squeak/Pharo but, e.g., Qt (C++ Framework) offers standard containers that are thread-safe and reentrant without serious performance issues.

Ciao,
Marcel Taeumel

-----Ursprüngliche Nachricht-----
Von: [hidden email] [mailto:[hidden email]] Im Auftrag von Igor Stasenko
Gesendet: Samstag, 16. Oktober 2010 08:01
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] Re: Example of non-concurrent code :)

On 16 October 2010 08:55, Marcel Taeumel
<[hidden email]> wrote:
> So Set>>do: is not thread-safe. That's an interesting point. I think it would really improve the quality of the present API if all standard containers would be thread-safe. Is there already a project that tries to achieve this?
>

We are discussed this before.
My own opinion on it: do not try to make standard collections like Set
thread safe,
because it will lead to fine-grained locking, which will make image
crawl like a snail.
This is wrong direction.

You should use either specialized containers, like SharedQueue,
or protect your data from concurrent access at more coarse-grained level.

--
Best regards,
Igor Stasenko AKA sig.