Still celebrating National IT Professionals Day with 3 months of free Premium Membership. Use Code ITDAY17

x
?
Solved

Singleton...sorta

Posted on 2006-11-27
19
Medium Priority
?
455 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
[X]
Welcome to Experts Exchange

Add your voice to the tech community where 5M+ people just like you are talking about what matters.

  • Help others & share knowledge
  • Earn cash & points
  • Learn & ask questions
  • 8
  • 6
  • 3
  • +1
19 Comments
 
LVL 86

Expert Comment

by:jkr
ID: 18021516
>>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
ID: 18021543
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
ID: 18021561
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
VIDEO: THE CONCERTO CLOUD FOR HEALTHCARE

Modern healthcare requires a modern cloud. View this brief video to understand how the Concerto Cloud for Healthcare can help your organization.

 
LVL 30

Expert Comment

by:Axter
ID: 18021652
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
ID: 18022634
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
ID: 18022688
>>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
ID: 18023293
>>>>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
ID: 18023321
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
ID: 18023338
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
 
LVL 4

Author Comment

by:Raymun
ID: 18025223
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
ID: 18025286
>>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
ID: 18025294
>>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
ID: 18025615
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
ID: 18025628
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 2000 total points
ID: 18025649
>>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
ID: 18029300
The named mutex worked. Thanks!
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 18029546
>>>> 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
ID: 18033408
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
ID: 18036256
>>>> 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 Tool: SSL Checker

Scans your site and returns information about your SSL implementation and certificate. Helpful for debugging and validating your SSL configuration.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

When writing generic code, using template meta-programming techniques, it is sometimes useful to know if a type is convertible to another type. A good example of when this might be is if you are writing diagnostic instrumentation for code to generat…
Basic understanding on "OO- Object Orientation" is needed for designing a logical solution to solve a problem. Basic OOAD is a prerequisite for a coder to ensure that they follow the basic design of OO. This would help developers to understand the b…
The viewer will learn how to pass data into a function in C++. This is one step further in using functions. Instead of only printing text onto the console, the function will be able to perform calculations with argumentents given by the user.
The viewer will learn additional member functions of the vector class. Specifically, the capacity and swap member functions will be introduced.
Suggested Courses

670 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