RE: patch for sched_yield()

["Bossom, John" <John dot Bossom at Cognos dot COM>]

Sorry, but I do not believe this patch is thread safe...

You have used a static variable (i.e. read a private global variable) declared within the function but did not provide any locking strategy around the initial assignment. On a true multi-processor machine where multiple threads call sched_yield() at precisely the same time, you may end up with garbage in the variable.

1) there may be a race condition to actually assign the initial value of the variable,
   depending on how the compiler generates the assignment to zero. If it is deferred
   until the method is actually called, then it would have generated a hidden test
   to see if this is the first time the method is called in order to perform the
   assignment once. This "hidden" test is not thread safe, if generated.

   Recommendation: move the static to the global scope so that the code generation
                   is performed either when the object file was compiled, or when
                   the code is initially loaded, but before user functions are declared.

2) Your test for zero should be followed by a lock to protect the variable and then 
   re-test for zero and then perform the assignment.

	i.e.)
		if( variable == 0 ){
			LOCK( m );
			if( variable == 0 ){
				variable = value;
			}
			UNLOCK( m )
		}

John. 

-----Original Message-----
From: pthreads-win32-owner@sourceware.org [mailto:pthreads-win32-owner@sourceware.org] On Behalf Of Virgilio Alexandre Fornazin
Sent: Wednesday, August 22, 2007 4:39 PM
To: pthreads-win32@sourceware.org
Subject: patch for sched_yield()

Hi

I did a change in sched_yield() to support multi-processor systems after checking the OS (NT style oses). I managed to compile it under VS2005 SP1.
I'm putting the code here to share with pthreads project.

I changed the implementation from:

{
  Sleep (0);

  return 0;
}


To:

{
  static OSVERSIONINFO vs = {0};

  if (vs.dwOSVersionInfoSize == 0)
  {
    vs.dwOSVersionInfoSize = sizeof(vs);
  
    GetVersionEx(&vs);
  }

  if (vs.dwPlatformId == VER_PLATFORM_WIN32_NT)
  {
    SwitchToThread ();
  }
  else
  {
    Sleep (0);
  }

  return 0;
}

I saw in another mailing list that Sleep(0) sometimes doesn't yield the thread and performs nothing on some Windows versions.

I hope this help somebody and could be integrated to main code after more testing with other cpu/configurations.


      Flickr agora em português. Você clica, todo mundo vê.
http://www.flickr.com.br/
 
     This message may contain privileged and/or confidential information.  If you have received this e-mail in error or are not the intended recipient, you may not use, copy, disseminate or distribute it; do not open any attachments, delete it immediately from your system and notify the sender promptly by e-mail that you have done so.  Thank you.