[sldev] [Fwd: [sldev-commits] Successful Build for http-texture (2235)]

[Dzonatas]

Here is an example of a race condition that sticks out like a sore 
thumb. We'll take the example from the recent changeset:
http://svn.secondlife.com/trac/linden/changeset/2232

In there it shows this code: mRawImage.notNull() && mRawImage->getDataSize()

That kind of function call, to test for null first then call the 
function, is found in many places in the source. The comment provides a 
hint the race condition happens in such function call, but given that 
style of call still exists in the code, I don't think it is truly 
understood. One should not need a stack trace to realize that the 
function call above will cause crashes. Sure, there are ways to 
circumvent it by clever uses of flags.

One thing I like about C# with delegates is that you get the speed of a 
function call without locks, like above, but also can eliminate the race 
condition with a simple step and not resort to locks. For example, in C#:

delegate void                      DataSizeHandler() ;
public static event DataSizeHandler      DataSize;
...
if( DataSize )
   DataSize()


That code above causes race conditions. If DataSize is not assigned to 
some valid function, which would make it null, it should not be called. 
First, let's assign it to a valid function:

DataSize += RawImage.DataSize;

So, now DataSize is set to non null, the if-condition lets the call 
happen. Along comes another thread that change the value of DataSize 
after the if-condition test true but before the call to DataSize():

DataSize -= RawImage.DataSize;

We can see now how the attempt to make the function call to DataSize 
will still crash and how the if-condition was a false sense of security 
that it wouldn't crash -- maybe avoided when another thread would 
execute in between the if-condition and the call. We know threads 
execute anytime and the possibility will happen. We need to eliminate 
that possibility.

In C#, there is an easy way to make it so this never ever crashes:

delegate void                      DataSizeHandler() ;
public static event DataSizeHandler      DataSize                 = 
delegate { } ;
...
DataSize()

The difference here is the use of an anonymous delegate. Events in C# 
are multicast, so if DataSize is not assigned with RawImage.DataSize, 
then DataSize will still call the empty anonymous function. 
Anotherwords, it's never null. Since it is never null, there is never a 
crash for because of a null value. The other threads can add and remove 
function assignments anytime without locks and never cause a crash.

Now I'm sure you may say, but that is C# and not C++. Yes, this example 
I made is in C#. What is needed in C++ is something *like* C# delegates 
(with anonymous delegates), which would solve these race conditions. 
Maybe google some examples, but the key thing that needs to happen is 
the lockless assign/deassign, and instead of a null value, it points to 
a valid function. It could be a dummy function. Being that the dummy 
function would require the same parameters, no doubt that every one of 
these functions calls like mRawImage->getDataSize() starts to look more 
like a need for a template class to wrap each declaration.

Cheers

Rob Lanphier wrote:
> This one seems to fix VWR-12775 in my limited testing....please give
> this one a spin!
> -------- Original Message --------
> Subject: 	[sldev-commits] Successful Build for http-texture (2235)
> Date: 	Fri, 8 May 2009 18:03:56 -0700 (PDT)
> From: 	buildadmin at lindenlab.com
> To: 	sldev-commits at lists.secondlife.com
>
>
>
> CYGWIN:
> http://secondlife.com/developers/opensource/downloads/2009/http-texture/2235/Second_Life_1-23-0-2235_OSS_Setup.exe
> http://secondlife.com/developers/opensource/downloads/2009/http-texture/2235/good-build.CYGWIN
>
> Darwin:
> http://secondlife.com/developers/opensource/downloads/2009/http-texture/2235/SecondLife_1_23_0_2235_OSS.dmg
> http://secondlife.com/developers/opensource/downloads/2009/http-texture/2235/good-build.Darwin
>
> Linux:
> http://secondlife.com/developers/opensource/downloads/2009/http-texture/2235/SecondLife-i686-1.23.0.2235.tar.bz2
> http://secondlife.com/developers/opensource/downloads/2009/http-texture/2235/good-build.Linux
>
> ------------------------------------------------------------------------
> r2235 | merov.linden | 2009-05-08 14:28:17 -0700 (Fri, 08 May 2009) | 1 line
> Changed paths:
>    M /projects/2009/http-texture/indra/newview/lltexturefetch.cpp
>
> VWR-12775: committed a simple patch that circunvents (but doesn't fix...) the most gregarious crashers
> ------------------------------------------------------------------------
> _______________________________________________
> Click here to unsubscribe or manage your list subscription:
> /index.html-commits
>
> _______________________________________________
> Policies and (un)subscribe information available here:
> http://wiki.secondlife.com/wiki/SLDev
> Please read the policies before posting to keep unmoderated posting privileges
>
>
>