multithreading not working as expected

[ShiroAisu]

#include <iostream>
#include <windows.h>
#include <process.h>

int x = 0, y = 0;

void myFunc( void* arg ) {

    for(;;) {
        ++x;
        ++y;
    }

}

int main() {

    _beginthread( myFunc, 0, 0 );

    for(;;) {
        std::cout <<x <<" " <<y <<"\n";
    }

}

My problem is that x and y are always 0, but if i had a sleep() to the threads entry function:

void myFunc( void* arg ) {

    for(;;) {
        ++x;
        ++y;
        Sleep(100); //Or even Sleep( 1 )
    }

}

x and y change correctly. Why is that?

Just to make it clear, ive just started with multithreaded programming.

[webJose]

I have no idea why Sleep() resolves the problem because I think your problem is compiler optimization; or I would have thought that.

You see, in your main() there is no place where x or y change, so the compiler could using that as an opportunity to optimize the code and making the loop not check for value changes in the variables. This is at least, what I thought it was.

My solution would have been to declare the variables volatile:

volatile int x;
volatile int y;

You could try it, but I don't see how this relates to a Windows API like Sleep(). :-(

[kbw]

You never really give the scheduler a chance to run the thread. When you call Sleep, you suspend the main thread, giving the worker thread a chance to do something.

These are synchronised event that you are not synchronising. What you have is called a race condition and the results are non-deterministic.

The sequence should be:
1. MainThread: initialise x, y
2. MainThread: Start worker thread
3. MainThread: wait to be signalled
4. WorkerThread: increment x, y
5. WorkerThread: signal MainThread
6. WorkerThread: wait to be signalled
7. MainThread: print x, y
8. repeat 3.

Of course you're not doing that.

[webJose]

I understand your logic, kbw, but Windows is a preemptive OS. Regardless of whether main() doesn't stop, at some point Windows will switch thread contexts sooner or later, the other thread is bound to run. Or am I missing something?

[ShiroAisu]

Thanks for the replies, i want to say that i know of the things you guys told me, i know i better use sync for theese things and i know what volatile does and that it infact fixes it.

The reason because im not using neither in this piece of code is because im trying to figure out WHY ON EARTH does the main thread check for those variables values when i put some random funcion like std::cout for example ( or sleep like i said before) on the second thread's loop. I cant see the logic behind it :\

Does someone have an idea about WHY this happens??

[kbw]

The scheduler parks you when you do I/O or Sleep, passing control to something else. So it changes the order in which the threads get run and for how long. That's why you need to synchronise synchronised tasks.

[webJose]

I understand that. But it is not required to switch thread contexts. It is just A method of forcing a switch. If an application's thread runs an infinite loop, Windows will eventually switch it to other threads because it is the very definition of a preemptive OS. This means that the other thread is bound to run even without a Sleep() call.

Also note that Sleep() is added to the worker thread, yet it is the main thread that shows all zeroes, meaning it is running, so Sleep()'s behavior doesn't really explain why this overall odd behavior is happening.

[webJose]

I keep thinking about this, and I just can't figure it out. Maybe it is just a plain old bug in the MS compiler. If you know assembly, compare the assembly generated in either case, and if it makes no sense, post a bug report @ connect.microsoft.com.

[ShiroAisu]

well, i dont really believe it is a bug, nobody trips on his/her keyboard and writes something that makes all of the program's threads recheck the values of their variables accidentaly...

I dont really have much assembly experince so i cant test that theory :\ but it would be awsome if somebody did.

sory, i forgot to mention: im using minGW

[kfmfe04]

kbw is right

unless windows threading is very different from standard threading, you seriously need a mutex around shared variables x and y every time you want to read or modify their values.

doing what you are doing in, say, pthreads, would result in unpredictable race conditions, pretty much what you are observing now, in windows

[webJose]

This is not a synchronizing issue, people. Synchronization has nothing to do with the fact that making the variables volatile the output shows the expected non-zero values. The issue is something else.

[ShiroAisu]

Ok, i got a nice answer that explains whats going on:

this is from Codeplug on the cprogramming forums:

It didn't "work". You did it wrong two different ways and observed different results. If you want to explore undefined behavior, then you need to look at the assembly code emitted by your compiler, and understand what it means on your particular hardware.

In this case however, I can guess the "why". The compiler optimized accesses to x and y by placing them into CPU registers. When you introduced the call to Sleep, the compiler could not guarantee that the innards of Sleep don't modify x and y, since they have external linkage. Because of this, the compiler could not place x and y into registers.

This explains it :) thank you for all your help everyone.

[kfmfe04]

there apparently are Microsoft-specific semantics to volatile that make it behave like a mutex:

http://msdn.microsoft.com/en-us/library/12a04hfd(v=vs.80).aspx

but that is Visual Studio - I have no idea how applicable that is to minGW

warning: I am pretty certain that just doing volatile would be insufficient on a gcc/POSIX system

anyhow, volatile is a side-track off of the OP - whatever compiler you use, you do need some form of mutex to get predictable, synchronized behavior on your OS

[webJose]

Ah, that's very interesting. I tried to create threads in ideone.com, but since that runs in Linux, I could not because I don't know how to create threads in Linux. :-(

Tried pthread.h, but could not make it work.

[kfmfe04]

well, the problem with ideone.com isn't pthread.h - it's that there appears to be no way to pass -lpthread to the linker

if you have your own POSIX system set up, you can try something like this:

#include <pthread.h>
#include <iostream>

#include <stdlib.h>
#include <time.h>

using namespace std;

static pthread_mutex_t counter_mx = PTHREAD_MUTEX_INITIALIZER;
static pthread_cond_t  zero_cv    = PTHREAD_COND_INITIALIZER;
static int             counter    = 10;
static unsigned int    seed       = clock(); 

static void *countdown_cb( void *v )
{
  const char *name = reinterpret_cast<const char*>( v );
  bool done  = false, needToBroadcast = false;
  int  usecs = 0;

  while ( true ) {
    pthread_mutex_lock(   &counter_mx );
      if ( (done = ( counter==0 )) ) break;
      usecs = (int)(10.0 * rand_r( &seed ) / RAND_MAX) + 1;
      needToBroadcast = (--counter==0);
      fprintf( stderr, "[%s] %d\n", name, counter );
    pthread_mutex_unlock( &counter_mx );
    usleep( usecs );
  }
  if ( needToBroadcast )
     pthread_cond_broadcast( &zero_cv    );
  pthread_mutex_unlock(   &counter_mx );
  return NULL;
}

static void *waiting_cb( void *v )
{
  const char *name = reinterpret_cast<const char*>( v );
  bool        done = false;
  while ( !done ) {  
    pthread_mutex_lock(   &counter_mx );
      fprintf( stderr, "[%s] waiting\n", name );
      // pthread_cond_timedwait() for non-blocking
      pthread_cond_wait(    &zero_cv, &counter_mx );
      done = ( counter==0 );
      if ( done ) fprintf( stderr, "[%s] done\n", name );
    pthread_mutex_unlock( &counter_mx );
  }
  return NULL;
}

int main()
{
  pthread_t a, b, c, d;

  pthread_create( &d, NULL, waiting_cb,   const_cast<char*>( "thread d" ));
  pthread_create( &a, NULL, countdown_cb, const_cast<char*>( "thread a" ));
  pthread_create( &b, NULL, countdown_cb, const_cast<char*>( "thread b" ));
  pthread_create( &c, NULL, countdown_cb, const_cast<char*>( "thread c" ));

  pthread_join( a, NULL );
  pthread_join( b, NULL );
  pthread_join( c, NULL );
  pthread_join( d, NULL );

  return 0;
}

[webJose]

So you are saying that pthread is THE way to create multiple threads in Linux? No other way to do it?

[modoran]

Why are you talk about Linux threads in windows forum ? The windows way is CreateThread() exported from kernel32.dll, _beginthread() or _beginthreadex() implemented in Microsoft C Runtime Library (MSVCRT.DLL).


And yes, pthreads (POSIX threads) is the Linux library for multithreading.

[webJose]

Read the topic and find out why I asked about Linux: I wanted to try this out @ ideone.com.