I've found an interesting multithreading issue which is related not only to
ICS component but many many other components I've seen so far. It is a race
condition between two thread where one is triggering an event handler and
the other is removing (setting to nil) the event handler.
Consider the following procedure:
Line1: procedure TMyComponent.TriggerMyEvent(MyArg : Integer);
Line2: begin
Line3: if Assigned(MyEvent) then
Line4: MyEvent(Self, MyArg);
Line5: end;
Imagine this is used in a context where the component run in thread #1 and
where thread #2 change the MyEvent event handler to nil just when thread
#1 has evaluated the expression at line 3 as true, experienced a context
switch and come back up and running at line 4. At that time MyEvent is nil
and you get an exception !
The solution is to rewrite the procedure as follow:
Line1: procedure TMyComponent.TriggerMyEvent(MyArg : Integer);
Line2: var
Line3: TMyEventProc EventProc;
Line4: begin
Line5: EventProc := MyEvent;
Line6: if Assigned(EventProc) then
Line7: EventProc(Self, MyArg);
Line8: end;
Saving the event pointer in line5 make sure that it is still valid in the
case a thread switch between Line 6 and 7 occur and the MyEvent is set to
nil by the other thread.
Interesting, isn't it ?
Contribute to the SSL Effort. Visit

>I've found an interesting multithreading issue which is related not only
>to
>ICS component but many many other components I've seen so far. It is a
>race
>condition between two thread where one is triggering an event handler and
>the other is removing (setting to nil) the event handler.
>[]
>
Quite normal. The proper way to disable a handler is not nil'ing it nor
grabbing and saving pointer to handler proc and then calling it (because
then the thread1 may do something that thread2 don't want do), but using
a global, protected by critical section variable.

Sorry but I don't agree with you. The only proper way to disable an event
handler is to nilling it. Using a global variable is a very bad design or
even wrong design: you may have many instances of the same component.
Protecting using a critical section doesn't solve anything in that case and
would have a negative impact on performance. It is thread's #2
responsability to handle the consequences of nilling the event handler. The
component having an handler nilled can trigger an null pointer exception
because of that but is not responsible for what happend outside of his own
code.

But the best solution is to not nil any handler in another thread, ie. use
a logic that doesn't require such actions.

That is not what I call "a best solution". This is what I call a "work
around".
The best solution is mine.

procedure TMyComponent.TriggerMyEvent(MyArg : Integer);
begin
EnterCriticalSection(SomeSection);
if Assigned(MyEvent) then
MyEvent(Self, MyArg);
LeaveCriticalSection(SomeSection);
end;

Now we're certain that nothing would change MyEvent between the
"ifthen" and MyEvent call, and old MyEvent won't be triggered even
if thread2 would nil it in-between ifthen and MyEvent - thread2 will
nil it AFTER MyEvent occurence.

Wrong.
You have to use the same critical section everywhere you assign a value to
MyEvent. Better write a setter for that purpose.

>It is thread's #2
>responsability to handle the consequences of nilling the event handler.
>The
>component having an handler nilled can trigger an null pointer exception
>because of that but is not responsible for what happend outside of his
>own
>code.
>
True, but what if thread2 nils the event handler AFTER thread1 saves the
event handler's procedure address? Thread2 may do this because of
something
happened - something that requires the event to NT occur.

That is why I said: "It is thread's #2 responsability to handle the
consequences".
The developer using the component has to manage HIS code so that there is no
race condition. The component developer has to manage HIS code so that there
is no null pointer exception when the reace condition occur.

Your solution isn't bad, it simply can cause some confusion if someone
relies on events not occuring.

The race condition is caused by the component user, not the component
developer.
In my opinion, the responsibilities are:
1) The component developer has to manage to not crash for race condition
where the component user can't handle it.
2) The component user has to manage the race condition.

Hello Francois,
Line3: if Assigned(MyEvent) then
Line4: MyEvent(Self, MyArg);
Very interesting issue. Because almost all components are coded like
this. Very interesting solution also, clean and simple :)
Rgds, Wilfried [TeamICS]
http://www.mestdagh.biz

Francois PIETTE wrote:
I've found an interesting multithreading issue which is related not
The solution is to rewrite the procedure as follow:

Line1: procedure TMyComponent.TriggerMyEvent(MyArg : Integer);
Line2: var
Line3: TMyEventProc EventProc;
Line4: begin
Line5: EventProc := MyEvent;
Line6: if Assigned(EventProc) then
Line7: EventProc(Self, MyArg);
Line8: end;
Saving the event pointer in line5 make sure that it is still valid in
the case a thread switch between Line 6 and 7 occur and the MyEvent
is set to nil by the other thread.

Interesting, isn't it ?

I think it's better/faster than having critical sections for all triggers,
do you plan to change all ICS triggers accordingly?

Arno Garrels [TeamICS]

Arno Garrels wrote:
Francois PIETTE wrote:
>I've found an interesting multithreading issue which is related not
>The solution is to rewrite the procedure as follow:
>
>Line1: procedure TMyComponent.TriggerMyEvent(MyArg : Integer);
>Line2: var
>Line3: TMyEventProc EventProc;
>Line4: begin
>Line5: EventProc := MyEvent;
>Line6: if Assigned(EventProc) then
>Line7: EventProc(Self, MyArg);
>Line8: end;
>Saving the event pointer in line5 make sure that it is still valid in
>the case a thread switch between Line 6 and 7 occur and the MyEvent
>is set to nil by the other thread.
>
>Interesting, isn't it ?

I think it's better/faster than having critical sections for all
triggers, do you plan to change all ICS triggers accordingly?

This requires atomic reading/writing of a Pointer (no thread switch
while read/write), I'm not sure whether this is true. Byte variables
are written/read in one go, there's no ploblem.

Arno Garrels [TeamICS]

Hello Arno,

This requires atomic reading/writing of a Pointer (no thread switch
while read/write), I'm not sure whether this is true. Byte variables
are written/read in one go, there's no ploblem.

As far as I know it is also the case with a 32 bit. So should be no
problem.

Rgds, Wilfried [TeamICS]

http://www.mestdagh.biz

Message
From: "Wilfried Mestdagh" <wilfried (AT) mestdagh (DOT) biz>
To: "ICS support mailing" <twsocket (AT) elists (DOT) org>
Sent: Monday, August 07, 2006 10:40 AM
Subject: Re: [twsocket] Interesting multithreading issue:
raceconditionwhentriggering an event handler

: Hello Arno,
:
: This requires atomic reading/writing of a Pointer (no thread switch
: while read/write), I'm not sure whether this is true. Byte variables
: are written/read in one go, there's no ploblem.
:
: As far as I know it is also the case with a 32 bit. So should be no
: problem.

I think it is related with what the machine "word" is. For Athlon 64, it is
64 bits!

Regards,

SZ

This requires atomic reading/writing of a Pointer (no thread switch
while read/write), I'm not sure whether this is true.

I think so since pointer are 32 bits and are accessed by only one memory
cycle. If you look at the Windows API InterlockedXYZ function, there is none
to access a single 32 bit value. So I guess it is guaranteed to have a 32
bit access done in one chunk.

Contribute to the SSL Effort. Visit

Francois Piette wrote:
>This requires atomic reading/writing of a Pointer (no thread switch
>while read/write), I'm not sure whether this is true.

I think so since pointer are 32 bits and are accessed by only one
memory cycle. If you look at the Windows API InterlockedXYZ function,
there is none to access a single 32 bit value. So I guess it is
guaranteed to have a 32 bit access done in one chunk.

In order to clear myself up finally I goggled a bit and found this:

"Simple reads and writes to properly-aligned 32-bit variables are atomic. In other words, when one thread is updating a 32-bit variable, you will not end up with only one portion of the variable updated; all 32 bits are updated in an atomic fashion. However, access is not guaranteed to be synchronized. If two threads are reading and writing from the same variable, you cannot determine if one thread will perform its read operation before the other performs its write operation.
Simple reads and writes to properly aligned 64-bit variables are atomic on 64-bit Windows. Reads and writes to 64-bit values are not guaranteed to be atomic on 32-bit Windows. Reads and writes to variables of other sizes are not guaranteed to be atomic on any platform."

But is this also true for Unix S? Some articles I found say that atomicity
is garanteed up to native int only.

Arno Garrels [TeamICS]

Contribute to the SSL Effort. Visit

This raise the following question : Is a field variable in a class aligned
in memory ? (MyEvent variable is just a field variable).

This raise the following question : Is a field variable in a class
aligned in memory ? (MyEvent variable is just a field variable).

I don't know, and it also raises a second question:

Reads and writes to variables of
other sizes are not guaranteed to be atomic on any platform."

Are i.e. reads/writes of Byte-variables not atomic in any Windows
version, and if yes what versions are they? If that was true for
32-Bit Win I would have to change plenty of code :(

Anything shorter than a 32 bit integer and properly aligned is always
atomic. As a byt can't be split, it is always read/written atomicly.