Singleton...sorta

>>My goal is to allow only one thread access to pRay

Are you thinking of something like

class Ray
{
     static Ray* pRay;
     static bool bLocked

    public:
     static Ray* GetInstance();
     static void ReleaseInstance() {bLocked = false;};

    protected:
     Ray() : bLocked(false);
};

Ray* Ray::pRay = 0;

Ray* Ray::GetInstance()
{
     if (bLocked) return NULL;

     if( pRay == 0 ) {
          pRay = new Ray;
   
      }

     bLocked = true;

     return pRay;
}

?

BTW: A better alternative would be a synchronization lock (mutex, semaphore) that is aquired in 'GetInstance()' and released via e.g. 'ReleaseInstance()'.

FYI:
If you are think about similar logic to what jkr posted, be aware that the above Getinstance function is not thread safe.

You need to use a mutex or critical_section logic, to make it thread safe.
What OS are you using, and what' s your compiler?

I recommend using a synchronize smart pointer.
See following link:
http://axter.com/smartptr/

Or a simplified version:
http://axter.com/smartptr/classsync__ptr.htm

I am using VC++ on XP.

To my understanding, critical_section is faster than mutex. What other differences are there? Which is better for my situation?

I tried implementing critical_section:

#include <windows.h>

class Ray
{
     static Ray* pRay;
     CRITICAL_SECTION* cs;

    public:
     static Ray* GetInstance();

    protected:
     Ray();
};

Ray* Ray::pRay = 0;
CRITICAL_SECTION* cs = 0;

Ray* Ray::GetInstance()
{
    ::TryEnterCriticalSection( cs );

     if( cs == 0 ) {
          pRay = new Ray;
          return pRay;
     }

     return 0;
}

...but I get an error 'TryEnterCriticalSection': is not a member of 'operator` `global namespace'"

If I try it without the double-colon I get a different error 'TryEnterCriticalSection': identifier not found...

>>To my understanding, critical_section is faster than mutex. What other differences are there?

CRITICAL_SECTIONs can only be used to sync threads within one process, the others work over process boundaries also. BTW, that should be more like

class Ray
{
     static Ray* pRay;
     static CRITICAL_SECTION cs;

    public:
     static Ray* GetInstance();
     static void ReleaseInstance() {LeaveCriticalSection(&cs);};

    protected:
     Ray() :{InitializeCriticalSection(&cs)};
};

Ray* Ray::pRay = 0;
CRITICAL_SECTION Ray::cs;


Ray* Ray::GetInstance()
{
     EnterCriticalSection(&cs);

     if( pRay == 0 ) {

          pRay = new Ray;
   
      }

     return pRay;
}

>>>>To my understanding, critical_section is faster than mutex.

In my testing, I've found a FastMutex to be even faster then critical_section logic.
Here's the code for a FastMutex class.

class FastMutex
{
      long locker;
public:
      FastMutex() : locker(0) {}
      void lock()
      {
            // check if another thread already locked
            while (InterlockedIncrement(&locker) > 1)
            {
                  InterlockedDecrement(&locker);
                  Sleep(rand()%3);   // sleep 0 to 2 milliseconds before next attempt
            }
      }      
      void unlock() {  InterlockedDecrement(&locker); }
private:
      FastMutex(const FastMutex&); //not copiable
      FastMutex& operator=(const FastMutex&);//not copiable
};

Example usage:
lass Ray
{
     static Ray* pRay;
     static FastMutex fastmutex;
    public:
     static Ray* GetInstance();
};

Ray* Ray::GetInstance()
{
     fastmutex.lock();

     if( pRay == 0 ) {
          pRay = new Ray;
     }

     fastmutex.unlock();
     
     return pRay;
}

FYI:
It's safer to declare your static pointer for your singleton inside a static function, then  it is to declare it outside the static function.
The following is a safer singleton method.
class Ray
{
    public:
     static Ray* GetInstance();
};

Ray* Ray::GetInstance()
{
     static Ray* pRay = NULL;
     static FastMutex fastmutex;
     fastmutex.lock();

     if( pRay == NULL) {
          pRay = new Ray;
     }

     fastmutex.unlock();
     
     return pRay;
}

jkr:
It crashes and I think it's because EnterCriticalSection is being called before InitializeCriticalSection. How can I fix this?

Axter:
I put fastmutex.unlock() in a static ReleaseInstance method. Is that right? I didn't think that call should be in GetInstance(). Anyway I tried it and it is still running a second instance.

I think maybe you guys should know how I'm testing it:

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

#include "Ray.h"

int main( int argc, char* argv[] )
{
      Ray* pRay = Ray::GetInstance();
      
      std::string s;
      std::cout << "Enter something: ";
      std::cin >> s;
      
      Ray::ReleaseInstance();
      
      return 0;
}

I open up a console and then run it. While that's waiting for input, I open up another console and run the same .exe. I want nothing to happen on the second console.

>>It crashes and I think it's because EnterCriticalSection is being called before InitializeCriticalSection. How can I fix this?

It's probably crashing because you have your static variables out side of the function, which means you can't be sure the the variables have been initialized before first use.

If you move the static variables inside the static function, you can then be sure that the variable is initialized.

Can you post the code  your testing using the fastmutex?

>>I want nothing to happen on the second console.
Then you need to lock it out side of the GetInstance function.

Thanks for you patience Axter. I don't understand. If I put the static variables into GetInstance, then ReleaseInstance won't be able to see them (in order to call LeaveCriticalSection). Anyways here's the code for fastmutex:

class Ray
{
      static Ray* pRay;
      static FastMutex fastmutex;
      
  public:
      static Ray* GetInstance();
      static void ReleaseInstance() { fastmutex.unlock(); }
            
  protected:
      Ray();
};

Ray* Ray::pRay = NULL;
FastMutex Ray::fastmutex;

Ray* Ray::GetInstance()
{
      fastmutex.lock();
      
      if( _Ray == 0 ) {      
            _Ray = new Ray;
      }
      
      return _Ray;
}

The same issue is here too. If I declare fastmutex in GetInstance(), then how do I call fastmutex.Unlock()?

Also, I haven't tried jkr's very first solution (using bool) but it definitely looks like it would work. Other than thread safety, why are mutex/fastmutex/critical_section better than it? With my setup, I wouldnt have to worry about thread safety because the .exe is being called at timed intervals.

The named mutex worked. Thanks!

>>>> because I have Task Scheduler calling it every hour
>>>> because the .exe is being called at timed intervals

The statements above make clear that the same executable is called multiple times.

Note, as Axter told you you can't share a singleton class object between multiple instances of the same executable.

The way out is either to use a named mutex as jkr and Axter suggested. At program start you would try to create a named mutex by CreateMutex (name it like your executable). If the create function returns with an mutex and GetLastError() equals to ERROR_ALREADY_EXISTS, another instance of your executable is still running. Then you could wait in a polling loop until the mutex was freed or you retrieve the instance handle of the running executable and use WaitForSingleObject to wait until it terminates. After that you newly can create the named mutex as owner.

Another method is to change the design of your current executable. Then, the scheduler wouldn't start a new instance of the executable but sends a message to the always running executable that it was scheduled again. If the program already has finished its job it would do it again. Else it would finish the current job and start a new run afterwards. If the job was done prior the new schedule message arrived, the program would wait for that message. The message can be a windows message if the program already has a (asynchronously running) message loop. Or it can be send via Windows sockets.


Regards, Alex

Thanks Alex. I did it using the first method you described. I Google-searched an example and it worked on the first try. Regarding your second method, it sounds good too but not with my situation. I have the app running every hour from 7-19 on weekdays, once a day on another machine, twice a day on another, etc etc. You get the point. It's much easier to conifgure the schedule from Task Manager than from within the code.

>>>> I did it using the first method you described

Yes, I also thought it would be easier. The second method is more suitable for a 'service' - where you need multi-threading any way - than for a normal executable.

Regards, Alex