Solved

Singleton...sorta

Posted on 2006-11-27
19
428 Views
Last Modified: 2010-04-01
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
Comment
Question by:Raymun
  • 8
  • 6
  • 3
  • +1
19 Comments
 
LVL 86

Expert Comment

by:jkr
Comment Utility
>>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
 
LVL 86

Expert Comment

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

Expert Comment

by:Axter
Comment Utility
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
 
LVL 30

Expert Comment

by:Axter
Comment Utility
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
 
LVL 4

Author Comment

by:Raymun
Comment Utility
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
 
LVL 86

Expert Comment

by:jkr
Comment Utility
>>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
 
LVL 30

Expert Comment

by:Axter
Comment Utility
>>>>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
 
LVL 30

Expert Comment

by:Axter
Comment Utility
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
 
LVL 30

Expert Comment

by:Axter
Comment Utility
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
Enabling OSINT in Activity Based Intelligence

Activity based intelligence (ABI) requires access to all available sources of data. Recorded Future allows analysts to observe structured data on the open, deep, and dark web.

 
LVL 4

Author Comment

by:Raymun
Comment Utility
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
 
LVL 30

Expert Comment

by:Axter
Comment Utility
>>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
 
LVL 30

Expert Comment

by:Axter
Comment Utility
>>I want nothing to happen on the second console.
Then you need to lock it out side of the GetInstance function.
0
 
LVL 4

Author Comment

by:Raymun
Comment Utility
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
 
LVL 4

Author Comment

by:Raymun
Comment Utility
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
 
LVL 30

Accepted Solution

by:
Axter earned 500 total points
Comment Utility
>>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
 
LVL 4

Author Comment

by:Raymun
Comment Utility
The named mutex worked. Thanks!
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
Comment Utility
>>>> 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
 
LVL 4

Author Comment

by:Raymun
Comment Utility
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
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
Comment Utility
>>>> 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

Featured Post

Free Trending Threat Insights Every Day

Enhance your security with threat intelligence from the web. Get trending threat insights on hackers, exploits, and suspicious IP addresses delivered to your inbox with our free Cyber Daily.

Join & Write a Comment

Templates For Beginners Or How To Encourage The Compiler To Work For You Introduction This tutorial is targeted at the reader who is, perhaps, familiar with the basics of C++ but would prefer a little slower introduction to the more ad…
Container Orchestration platforms empower organizations to scale their apps at an exceptional rate. This is the reason numerous innovation-driven companies are moving apps to an appropriated datacenter wide platform that empowers them to scale at a …
The goal of the video will be to teach the user the concept of local variables and scope. An example of a locally defined variable will be given as well as an explanation of what scope is in C++. The local variable and concept of scope will be relat…
The viewer will learn how to user default arguments when defining functions. This method of defining functions will be contrasted with the non-default-argument of defining functions.

771 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

10 Experts available now in Live!

Get 1:1 Help Now