• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 473
  • Last Modified:

Singleton...sorta

Hello, I have this:

class Ray
{
      static Ray* pRay;

    public:
      static Ray* GetInstance();

    protected:
      Ray();
};

Ray* Ray::pRay = 0;

Ray* Ray::GetInstance()
{
      if( pRay == 0 ) {
            pRay = new Ray;
            return pRay;
      }

      return 0;
}

I know Singleton is designed to return a common instance of an object but I want to modify it so that whenever GetInstance() is called, it will create and return an instance of pRay only if it is null. Otherwise, it will return null (instead of pRay). My goal is to allow only one thread access to pRay because I have Task Scheduler calling it every hour, but sometimes it runs for more than an hour and I don't want it to run another thread if it is already running. Right now my code is functioning like a real Singleton (returning the common instance of pRay) and not the way I want it to. How do I modify it so it returns null if pRay is not null? Or is there a better way to accomplish this?
0
Raymun
Asked:
Raymun
  • 8
  • 6
  • 3
  • +1
1 Solution
 
jkrCommented:
>>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;
}

?
0
 
jkrCommented:
BTW: A better alternative would be a synchronization lock (mutex, semaphore) that is aquired in 'GetInstance()' and released via e.g. 'ReleaseInstance()'.
0
 
AxterCommented:
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?
0
Cloud Class® Course: SQL Server Core 2016

This course will introduce you to SQL Server Core 2016, as well as teach you about SSMS, data tools, installation, server configuration, using Management Studio, and writing and executing queries.

 
AxterCommented:
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
0
 
RaymunAuthor Commented:
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...
0
 
jkrCommented:
>>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;
}
0
 
AxterCommented:
>>>>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
};
0
 
AxterCommented:
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;
}
0
 
AxterCommented:
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;
}
0
 
RaymunAuthor Commented:
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.
0
 
AxterCommented:
>>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?
0
 
AxterCommented:
>>I want nothing to happen on the second console.
Then you need to lock it out side of the GetInstance function.
0
 
RaymunAuthor Commented:
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()?
0
 
RaymunAuthor Commented:
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.
0
 
AxterCommented:
>>I wouldnt have to worry about thread safety because the .exe is being called at timed intervals.

Are you calling the same executable multiple times, and trying to lock it sone only one call has access to the application at one time?

If so, not only will jkr's code not work, but neither would the FastMutex nor critical_section logic.

You need a NAMED mutex to synchronize multiple instances of your application.

Jkr's method and the FastMutex method will not work because each instance of an application has it's own memory for the variables.
You would need to use a locking logic that can cross multiple instances of the application.  A named mutex can do that.
0
 
RaymunAuthor Commented:
The named mutex worked. Thanks!
0
 
itsmeandnobodyelseCommented:
>>>> 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

0
 
RaymunAuthor Commented:
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.
0
 
itsmeandnobodyelseCommented:
>>>> 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
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

Join & Write a Comment

Featured Post

Cloud Class® Course: Python 3 Fundamentals

This course will teach participants about installing and configuring Python, syntax, importing, statements, types, strings, booleans, files, lists, tuples, comprehensions, functions, and classes.

  • 8
  • 6
  • 3
  • +1
Tackle projects and never again get stuck behind a technical roadblock.
Join Now