Analysing Problem with Static Singleton

[Mohamed Farouk]

Guys
Please help, I am trying to figure out a problem in live with the following piece of code.

This is unusual coding style where in someone has created an Instance of an Singleton object using an local instance inside the method first and then finally assigning to static (Singleton) instance.

But what i find is Finalizer thread running frequently on these local instance which will destroy PinServer31 object which is not expected to happen by design

I understand that code should use static instance rather than local instance to sovle any problem.
But i am not able to explain why currently the below code makes local instances to be garbage collected please can you explain.

   
private PinServer31 _pinServer31;
private static PinServer CLASS_INSTANCE;
 
// Connect to the PIN Server
void connect() throws PinServerException {
     _pinServer31 = PinServer31.getInstance(run_mode, _customerSet);
}
 
public static synchronized PinServer getInstance() throws PinServerException {
        PinServer instance = CLASS_INSTANCE;
        if ((instance == null) || (instance._customerSet != customerSet)) {
                instance = new PinServer();
        }
        // Connect
        instance.connect();
        return (CLASS_INSTANCE = instance);
    }
 
 

[Ernest Friedman-Hill]

The only way you'd see instances being garbage collected is if getInstance() is returning a new instance of the class each time. This would happen if the condition (instance._customerSet != customerSet) wasn't true for the static instance -- either never true, or true at first but then becoming untrue. Check out whether getInstance() ever returns a new instance when the old one exists by adding print or log statements or by running the code in a debugger.

[sammaiah kyatham]

Hi,
One thing I can't understand, why do you need to assign it to local instance and compare..
I've slighly modified you code.. hope this will work for you!!!

private PinServer31 _pinServer31;
private static PinServer CLASS_INSTANCE;
// Connect to the PIN Server

void connect() throws PinServerException {
_pinServer31 = PinServer31.getInstance(run_mode, _customerSet);
}

public static synchronized PinServer getInstance() throws inServerException {

if ((CLASS_INSTANCE == null) {
CLASS_INSTANCE = new PinServer();
} // Connect
CLASS_INSTANCE.connect();
return CLASS_INSTANCE;
}

Thanks,
Sam

[Ilja Preuss]

Sam is right - the local variable doesn't serve any purpose.

The "instance._customerSet != customerSet" looks suspicious to me, too. Can you tell us more about what it might be intended to mean?

[Mohamed Farouk]

Hello Guys
Thanks for your replies, I as well dont understand the reason why the code was written like this. This is a existing piece of code in live which is causing trouble as I can see finalizer thread being run ever minute to clean up something which destroyes PinServer31 causing huge performance implications which I have found.

I am sure there is something wrong and as well I understand that the way to fix is to remove the local instance from the getInstance method.

What I dont understand is why is the local instance garbage collected every minute when there is a reference to it by the static instance?

I need to explain the problem as to why this code is incorrect, to fix it.

Please help! how will you reason out why it is incorrect and why does the local reference to be garbage collected often?

Thanks
Mohamed Farouk

[Mohamed Farouk]

Hello
Please do not worry about customer set , as i forgot to remove that line before posting it. The code in live contains that attribute as well but i thaught i can remove it for clarity. So please do not worry about it.

Thanks

[Mohamed Farouk]

Sorry Guys I figured out what is going on myself

 Cause of Concern : PinServer getInstance() method with Machine Instructions 
          The code implementation of singleton is the reason why there is Static PinServer  objects are garbage collected so many times causing huge performance implications leading to stale threads.
private PinServer31 _pinServer31; --- Declare a pointer to null memory area
private static PinServer CLASS_INSTANCE;  -- Declare a pointer to null area
 
public static synchronized PinServer getInstance(String mode, String customerSet) throws PinServerException {
     PinServer instance = CLASS_INSTANCE; -- Assign the address of  CLASS_INSTANCE to instance - Both point to the same memory area in heap
     run_mode = mode;
    logger.debug(_printName + "getInstance, customerSet=" + customerSet);
      if (customerSet != null) {   
      if ((instance == null) || (instance._customerSet != customerSet)) {  -       - Check if customer set is different from already exisiting one and  
         problem happens here when there is an existing instance
       instance = new PinServer();    --Allocate new memory are in heap and point the existing instance handle to point to the new object 
        }
       // All OK, so populate the constant
      instance._customerSet = customerSet; -- Set the customer set in the 
                                              new object 
       // Connect
      instance.connect();  -- Connect using new instance to create          PinServer31 (Singleton instances) and HttpConnectionManager 
 
           }
           return (CLASS_INSTANCE = instance);  - !!!!! ERROR  !!!!! Repoint instance handle pointing to new Object to the static object handle CLASS_INSTANCE. Both CLASS_INSTANCE to instance handles point to new object there by removing all the references to old object hence we see Finalizer being called to remove the old object references.
 
 }