Thread-safe Singleton

[Alan Morgan]

Hey all,

My Data class is a Singleton and while reading Head First Design patterns the other day (highly recommended by the way after I bought it on the Ranch's recommendation) I came across a section on thread-safe Singletons.

Silly me had never even considered needing to do this.

Anyway one solution is to synchronize the getInstance.
This has the most effect on performance
However it is only called when a new client is created and I reckon that will not happen often enough to make this a big issue.

So the fact that it is the simplest and I like the KISS principle means I wanna go for this option.

Obviously noted in choices doc.

Any good reasons not to do it this way ?

Cheers

[Pawel Poltorak]

Hi,

Synchronizing getInstance() is not the only solution. Look below:

public class MySingleton 
{
    private final static MySingleton instance = new MySingleton();
    private MySingleton() 
    {
        // construct the singleton instance
    }
    
    public static MySingleton getInstance() 
    {
        return instance;
    }
}

Here's a singleton which will be created when class is initialized, thread safe.

Best Regards,
Pawel

[Alan Morgan]

Hi,

Yes having a static initializer for the Singleton will work.

However I had been passing the databaseName in when creating the instance and obviously I wouldn't be able to do that in this case.

I could change it to set the databaseName rather than pass it in in creation however and it would allow me to do what is described above.

My first instinct was to go for path of least resistance which was to synchronize method.

[Ken Boyd]

What you guys think about code below which is singleton and also synz getInstance method..

/** Singleton declaration of Data
* class constructor
*/
private Data(){


}

/**
* method will return singleton instance of Data
* @return Data
*/
public static synchronized Data getInstance(){
if (uniqueData == null){
uniqueData = new Data();
}
return uniqueData;
}

[Alan Morgan]

Ken,

This is the method I was thinking of using.
It will work but will have most effect on performance.

The other method outlined by Pawel above solves the problem also.
But your solution will certainly work.

[Jonathan Moore]

Hi Alan,

Just to post my 2 cents, is there a definite reason why your Data class is a singleton?

I had my Data class as a singleton originally, but then it occurred to me that this may be limiting my design. The database file effectively represents a table, but what if in the future there were 2 or more database files (with different datasets) that could be accessed? The DB interface is generic and so the Data class can also be generic, but you would need more than one instance to access more than one database file, which can't be done if your class is singleton.

I decided not to make the class a singleton, and to pass the database file name into the Data constructor. When I open the file I attempt to lock it using FileChannel.tryLock(), and throw an exception if a lock already exists. This means I can have multiple Data instances, but only one for any given database file. Maybe I'm over-complicating the issue, but thought you may appreciate a different perspective.

Sorry if this is slightly off topic.

Cheers
Jon

[Christopher Bareja]

Hi,

public static Data getInstance(){
if (uniqueData == null){
synchronized(AN_OBJECT) {
if (uniqueData == null){
uniqueData = new Data();
}
}
}
return uniqueData;
}

Not big impact on performance (synch. block is entered only before singleton creation) and thread safe (without 2nd check singleton colud be created more than once)

[Paul Clapham]

That method is called "double-checked locking". Here's a link to a paper (written by people who know much more about the subject than I do) explaining why it doesn't work:

http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html

[Mihai Radulescu]

Hi,

Here are my two (euro)cents
1.Jonathan. is right the singleton is limiting the design - this thema was offen commented, just search for singleton.
2.The double-checked locking is not working always(look at http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html ). If you decide to use a singleton in multithread environment try to use it without lazy initialization, by example :

class Test {
    private static final instance = new Test();
 
    private Test(){
    }
 
    public static Test getInstance() {
         return instance;
    }
}

or with synchronized lazy init.

class Test {
    private static final instance;
 
    private Test(){
    }
 
    public static synchronized Test getInstance() {
         if (instance == null) {
             instance = new Test();
         }
 
         return instance;
    }
}

Regards
Mihai.

[Alan Morgan]

Jon,

Appreciate the different perspective.
So FileChannel.tryLock() locks the file (if not already locked) for the duration of its life ?

And then I assume each Data instance has its own set of lock maps or a lock manager if you have one ?

[Jonathan Moore]

Hi Alan,

I use a LockManager class, and have a single instance per Data instance (created in the Data constructer). Again I initially had LockManager as a singleton but would have needed to track which db file each lock was associated with. Otherwise if you try to lock record number 23 at the same time in two different files you'd get a SecurityException thrown for the second.

Cheers
Jon

[Jonathan Moore]

FileChannel.tryLock() will either return a FileLock object (if the file is not already locked) or null if the file is locked. If it returns null I throw an exception, meaning only a single instance of Data can access a file at a time.

The file can be unlocked by calling FileLock.release().

cheers
Jon

[Ken Boyd]

class Test {
    private static final instance = new Test();
 
    private Test(){
    }
 
    public static Test getInstance() {
         return instance;
    }
}

or with synchronized lazy init.

class Test {
    private static final instance;
 
    private Test(){
    }
 
    public static synchronized Test getInstance() {
         if (instance == null) {
             instance = new Test();
         }
 
         return instance;
    }
}

Regards
Mihai.[/qb]<hr></blockquote>


How can you declare

private static final instance;

because final variable value can't be alter in method..either you declare

private static instance;

and keep the code same for second solution...
[ November 15, 2005: Message edited by: Ken Boyd ]

[Alan Morgan]

Originally posted by Jonathan Moore:
Hi Alan,

Just to post my 2 cents, is there a definite reason why your Data class is a singleton?

I had my Data class as a singleton originally, but then it occurred to me that this may be limiting my design. The database file effectively represents a table, but what if in the future there were 2 or more database files (with different datasets) that could be accessed? The DB interface is generic and so the Data class can also be generic, but you would need more than one instance to access more than one database file, which can't be done if your class is singleton.

I decided not to make the class a singleton, and to pass the database file name into the Data constructor. When I open the file I attempt to lock it using FileChannel.tryLock(), and throw an exception if a lock already exists. This means I can have multiple Data instances, but only one for any given database file. Maybe I'm over-complicating the issue, but thought you may appreciate a different perspective.

Sorry if this is slightly off topic.

Cheers
Jon

Hi Jon,

I just got back to thinking about the point you made above.
I’m not sure I’ve gotten your point about multiple database files 100%.

I tried a test just there. I have two db files db1 and db2.
I start my application (alone mode) and link to db1.
Then I start another app (again alone mode) and link to db2.
This works fine and I can update both dbs separately.

Then I started thinking about server mode.
So I started 2 apps (both server mode) linked to db1 and db2 respectively.
When I started a network client app and connect to ip address of server, I am accessing whichever db I started second.

So I reckon the only way to connect to choose which server instance you want to connect to is to pass the db name in when you are connecting.
So you would need a db name field as well as an ip field when connecting in network client mode.

Is this correct or I am missing something ?

And if you are passing the db name in when connecting does this make sense for the client to be passing this in ?

[Jonathan Moore]

Hi Alan,

I'm referring to multiple data files accessed by the same application. The Data class can access any file that's in the correct format, so in the future it may be that there are 2 or more data files (containing different sets of data) - for instance you may have a data file containing details of all the customers. In that case you could create two instances of Data objects, one opening the customer file and one opening the hotel room file, from within the same application. If your class is a singleton you can't do that - you'd either need to modify your Data class or create another similar class that can open the new file.

OK, it's out of scope for this assignment, but I think if there's any possibility that you may need more than one instance of a class at any time you shouldn't make it a singleton.

Cheers
Jon

[Alan Morgan]

Hi Jon,

Right I have you now.
Your solution is the better option no doubt.
However at this point, being quite close to finished and even closer to sick of doing the project , I think the lesser of two evils may be to leave it as it is and note the limitations in the design choices doc.

As you say its probably outside the scope of the assignment so I shouldn't lose marks for making it a singleton.

As I say your option is the better one, but I reckon the reward at this point for changing is minimal (well hopefully)

Reasonable ?