Singleton Implementation, is this Singleton?

publicclass Wrapper{

publicstatic MyClass createInstance(){

returnnew MyClass();

}

}

I have seen this , this is not a singleton, since im creating a new MyClass, whenever i call the static method createInstance(). Am i right?

this is , right?

publicclass Singleton{

privatestatic Singleton instance =null;

publicstatic Singleton getInstance(){

if (instance ==null){

instance =new Singleton();

}

return instance;

}

private Singleton(){

}

}

[1471 byte] By [Cana_Bravaa] at [2007-10-2 10:48:37]
# 1

Close. It depends on how paranoid you are. Remember, the goal of the pattern is to assure that only one instance of the Singleton will exist:

public final class Singleton implements java.io.Serializable, Cloneable {

private static final Singleton INSTANCE;

static {

INSTANCE = new Singleton();

}

private Singleton() {

super();

if (INSTANCE != null) {

throw new IllegalStateException("Please do not use reflection to create me");

}

}

public static final Singleton getInstance() {

return INSTANCE;

}

public final Object readResolve() {

return INSTANCE;

}

public final Object clone()

throws CloneNotSupportedException {

throw new CloneNotSupportedException("Singleton should not be cloned");

}

}

Much of the above is normally optional. You really want a static and private instance variable and a private constructor. The rest is paranoia.

- Saish

Saisha at 2007-7-13 3:05:08 > top of Java-index,Other Topics,Patterns & OO Design...
# 2
You need to either create the instance on the same line it's declared, or else synchronize the getInstane method. Otherwise you're not threadsafe. You could end up with multiple instances, or with a corrupt instance.
jverda at 2007-7-13 3:05:08 > top of Java-index,Other Topics,Patterns & OO Design...
# 3
The first class is a factory in that it creates instances of an class.......The second one is a singleton.......
kcwilsona at 2007-7-13 3:05:08 > top of Java-index,Other Topics,Patterns & OO Design...
# 4

> You need to either create the instance on the same

> line it's declared, or else synchronize the

> getInstane method. Otherwise you're not threadsafe.

> You could end up with multiple instances, or with a

> corrupt instance.

Not with a static intiializer (above). Granted, only per JVM.

- Saish

Saisha at 2007-7-13 3:05:08 > top of Java-index,Other Topics,Patterns & OO Design...
# 5

> > You need to either create the instance on the same

> > line it's declared, or else synchronize the

> > getInstane method. Otherwise you're not

> threadsafe.

> > You could end up with multiple instances, or with

> a

> > corrupt instance.

>

> Not with a static intiializer (above). Granted, only

> per JVM.

I was referring to the OP's code. Guess I should've quoted.

jverda at 2007-7-13 3:05:08 > top of Java-index,Other Topics,Patterns & OO Design...
# 6
But why bother with the static init block if it's just doing the new Singleton() that you could've done on the declaration line?
jverda at 2007-7-13 3:05:08 > top of Java-index,Other Topics,Patterns & OO Design...
# 7

> You need to either create the instance on the same

> line it's declared, or else synchronize the

> getInstane method. Otherwise you're not threadsafe.

> You could end up with multiple instances, or with a

> corrupt instance.

About DCL (Double Checked Locking) problem, I think we can solve that without using synchronized method. I was reading some articles and I was thinking about it. I think we can do this:

public class Singleton {

private static Singleton.NestedClass auxNestedClass = null;

private static Object locker = new Object();

private Singleton() {

//maybe some things being executed here...

}

public static Singleton getInstance() {

if (auxNestedClass == null) {

synchronized (locker) {

if (auxNestedClass == null)

auxNestedClass = new Singleton.NestedClass();

return auxNestedClass.theSingleton;

}

//This "else if" below will return the theSingleton field only when

//this field is totally initialized (when the instance of this Singleton class is totally instantiated).

//It is guaranteed first because of the auxNestedClass field.

//If auxNestedClass is already initialized, so theSingleton field is already initialized, too.

//Besides, to guarantee even more, I decided to createc the controller boolean field.

} else if (auxNestedClass.controller) {

return auxNestedClass.theSingleton;

//This "else" below will be rarely executed.

//It磗 just to guarantee the return of theSingleton field.

} else {

try {

//We can wait for a period of time if we want,

//just to avoid a possible recursive execution of getInstance()

//in exagerated number of times.

Thread.sleep(10000);

} catch(InterruptedException e) {

e.printStackTrace();

}

return getInstance();

}

}

private static class NestedClass {

private Singleton theSingleton = null;

/* controller:

* This boolean variable will indicate that

* theSingleton is already initialized.

*/

private boolean controller = false;

/* When this constructor is done,

* we can be sure that theSingleton is initialized.

*/

private NestedClass() {

if (controller == false) {

theSingleton = new Singleton();

controller = true;

}

}

}

}

I am trying to guarantee a non-corrupt instance of Singleton using that private nested class. I think we can use this code for getInstance(), or getResource(), or any other kind of singleton pattern method. Well, untill someone in this forum discover some thing broken in my solution, but I don磘 see anything broken, and I think it can definitely solve the DCL problem.

PS: just to summarize, I磎 not considering reflection, or clone, or any other kind of "cheating". But I could have included those verifications, too.

Marcelo9a at 2007-7-13 3:05:08 > top of Java-index,Other Topics,Patterns & OO Design...
# 8

> About DCL (Double Checked Locking) problem, I think

> we can solve that without using synchronized method.

> I was reading some articles and I was thinking about

> it. I think we can do this:

Nope, sorry. That won't work. It's still DCL and it's still broken. You can still get multiple instances. Under the old memory model, you can also get a corrupted instance. The new MM might prevent the corrupted instance, I'm not sure.

jverda at 2007-7-13 3:05:08 > top of Java-index,Other Topics,Patterns & OO Design...
# 9

> Nope, sorry. That won't work. It's still DCL and it's

> still broken. You can still get multiple instances.

> Under the old memory model, you can also get a

> corrupted instance. The new MM might prevent the

> corrupted instance, I'm not sure.

I was reading this interesting article: [url http://www.javaworld.com/javaworld/jw-05-2001/jw-0525-double_p.html] Can double-checked locking be fixed?[/url], from JavaWorld. In this article, we can see some alternatives to solve the DCL problem:

Listing 2: Using a temporary variable

public Resource getResource() {

if (resource == null) {

synchronized {

if (resource == null) {

Resource temp = new Resource();

resource = temp;

}

}

return resource;

}

In Listing 2, you might think that as-if-serial semantics requires that the construction complete before resource is set, but that view is only from the perspective of the executing thread. Actually, the compiler is free to completely optimize away the temporary variable. Though numerous tricks have been suggested to prevent the compiler from optimizing away the temporary variable, such as making it public, the compiler can still vary the order in which assignments are made inside the synchronized block as long as the executing thread can't tell the difference.

In this code above, if I understood the problem correctly, the unsafe line is exactly this:

return resource;

Because we can磘 be sure that resource will be returned only after it is totally initialized. This resource can be corrupt. When the code is outside synchronized block, you are not able to guarantee the correct execution in a multithreaded application. So, we can try other alternatives. Let磗 see the next interesting code, although broken, too:

Listing 3: Using a guard variable

private volatile boolean initialized = false;

public Resource getResource() {

if (resource == null || !initialized) {

synchronized {

if (resource == null)

resource = new Resource();

}

initialized = (resource != null);

}

return resource;

}

At first, the approach taken in Listing 3 looks promising. Because initialized is set after the synchronized block exits, it appears that resource will have been fully written to memory before initialized is set. Listing 3 even attempts to ensure that initialized is not set until resource is set by making initialized's value depend on resource's value. However, synchronization doesn't work quite so literally. The compiler or JVM can move statements into synchronized blocks to reduce the cache-flush penalties associated with synchronization. But this means that from the perspective of other threads, initialized could still appear to be set before resource, and all of resource's fields are flushed to main memory.

The explanation of the author in this article is being gradually given to us. Very good. I think the objective of initialized variable here is to guarantee even more the safe return of resource, but unfortunately this solution is broken, too. If I understood correctly, it is broken because initialized can be set before resource, according to the explanation above. So you will have the same problem in the Listing 2. But, doesn磘 exist a way to safely verify the complete initialization of resource? I think it does, and it is exactly what I am intending to achieve in my code. But let磗 see the next listing from the article:

class FullMemoryBarrierSingleton {

private static boolean initialized = false;

private static Resource resource = null;

private static Object lock = new Object();

public static Resource getResource() {

if (!initialized) {

synchronized (lock) {

if (!initialized && resource == null)

resource = new Resource();

}

synchronized (lock) {

initialized = true;

}

}

return resource;

}

}

The current JMM does not permit the JVM to merge two synchronized blocks. So another thread could not possibly see initialized set before resource and all its fields are fully initialized and written to main memory. So have we solved the DCL problem? FullMemoryBarrierSingleton appears to avoid synchronization on the most common code path, without any obvious memory model hazards. Unfortunately, this is not exactly true.

Unfortunately, this solution is still broken, too.

I think the solution I am proposing can solve the problem because I am guaranteeing the complete initialization of singleton. Mainly because I am using the execution of constructor of other class to guarantee that. I guess it guarantees that because I am trying to prove through showing an absurd: If even a constructor cannot guarantee that, so we can conclude that all the constructors in Java are broken, which is obviously an absurd! If a constructor would be executed partially, then nothing worked correctly in java! You could take an instance partially contructed, and it would give you a lot of problems. Obviously, in Java you don磘 have this kind of problem (I hope!!!). Certainly Java doesn磘 permit you to use an partial object constructed. I guess that you can access a field of some object only after this object is completely constructed. Otherwise, all the JVM would be broken! This "theory of absurd" helps me in the solution I proposed. My solution is quite different from those of this article in JavaWorld. I would really appreciate if you showed me where exactly the code I propose is broken.

Marcelo9a at 2007-7-13 3:05:08 > top of Java-index,Other Topics,Patterns & OO Design...
# 10
Sorry, but I'm not going to read all that. Which code exactly is the code you propose? Can you post just that code please? Is it the same code I commented on earlier?
jverda at 2007-7-13 3:05:08 > top of Java-index,Other Topics,Patterns & OO Design...
# 11

> Sorry, but I'm not going to read all that.

>

> Which code exactly is the code you propose? Can you

> post just that code please? Is it the same code I

> commented on earlier?

Yes, it is the same (in reply 7).

Those other codes are from Javaworld article.

Marcelo9a at 2007-7-13 3:05:08 > top of Java-index,Other Topics,Patterns & OO Design...
# 12

public static Singleton getInstance() {

if (auxNestedClass == null) {

synchronized (locker) {

if (auxNestedClass == null)

auxNestedClass = new Singleton.NestedClass();

return auxNestedClass.theSingleton;

}

In the auxNestedClass = new line, the VM is allowed to first put the addres of the object into auxNestedClass and then initialize the object, because a single thread will be unable to tell the difference.

Under the memory model in JLS 2nd ed. (In effect for 1.2 (I think), 1.3 and 1.4, but not sure about 1.4.2), the first thread can execute the assignment to auxNested class, and then a second thread is allowed to see the non-null value of auxNestedClass after it's been set but before the constructor has completed.

Under the new MM in JLS 3rd ed., that might be prohibited. I'm not sure.

That's how you can get a corrupted instance.

What I said about possibly getting multiple instance was incorrect, I think, but I'm not sure.

I'm really not that interested in digging into DCL anymore. I used to care about finding a way to make it work, but that's really just an intellectual exercise. I don't think it's ever really necessary, and I think it's not possible to make it work correctly in Java, though the new MM might have made it possible.

jverda at 2007-7-13 3:05:08 > top of Java-index,Other Topics,Patterns & OO Design...
# 13
thanks for your replies.
Cana_Bravaa at 2007-7-13 3:05:08 > top of Java-index,Other Topics,Patterns & OO Design...
# 14

@JVerd: That's one of the most succinct and salient explanations of why double-checked locking is broken that I have read. :^)

Mainly, that's the reason I use the static intializers and keep the instance final. However, I have run into situations where this is not a viable option. For example, one might not want to initialize a connection pool (or other latent task) until it is actually necessary. (Then again, one could argue that if the pool is not being used that much, why pool?)

Ah. I'm not making sense. Off to breakfast.

- Saish

Saisha at 2007-7-13 3:05:08 > top of Java-index,Other Topics,Patterns & OO Design...
# 15

> @JVerd: That's one of the most succinct and salient

> explanations of why double-checked locking is broken

> that I have read. :^)

Thanks. I'm glad you found it useful.

> Mainly, that's the reason I use the static

> intializers and keep the instance final. However, I

> have run into situations where this is not a viable

> option. For example, one might not want to

> initialize a connection pool (or other latent task)

> until it is actually necessary. (Then again, one

> could argue that if the pool is not being used that

> much, why pool?)

I have a hard time imagining where this would make a difference.

The instance won't be created until the class is loaded. The class won't be loaded until it's referenced in the code. Where are you referencing it if not to use it? You'd have to be using some static method of the singleton class but not using the singleton instance. This seems like a highly unlikely (and probably poorly designed) situation.

jverda at 2007-7-20 20:52:46 > top of Java-index,Other Topics,Patterns & OO Design...
# 16

I have a hard time imagining where this would make a difference.

The instance won't be created until the class is loaded. The class won't be loaded until it's referenced in the code. Where are you referencing it if not to use it? You'd have to be using some static method of the singleton class but not using the singleton instance. This seems like a highly unlikely (and probably poorly designed) situation.

Well, specifically, (and this is legacy code), there is a Servlet context listener that references the object. Literally nothing happens at contextInitialized(), however a static debugging method is called. This would result in any static initializers on the pool class to fire. (Whether the above should happen in a Servlet context listener or whether the debugging method is truly needed are valid questions.) At contextDestroyed(), the pool is requested to close any open connections. So, until a getInstance() method is actually called, the initialization of the pool's connections are deferred. I do agree that a redesign could solve the issue.(Will be added to the very back burner).

- Saish

Saisha at 2007-7-20 20:52:46 > top of Java-index,Other Topics,Patterns & OO Design...
# 17

@Marcelo:

Your code does not solve it for two reasons:

1. Reordered writes.

2. Read/Write barrier.

Firstly, reordered writes means the compiler can under certain rules reorder the writes. It means a variable may be assigned before the object is initialized, it also means that variables can be assigned in different orders than they appear in the code. This leads to two distinct problems with your code.

First of all, the auxNestedClass can be assigned before it is initialized. Hence another thread can see auxNestedClass as non-null. You're hoping that looking at the boolean variable will solve this, but it won't. The controller variable can be assigned before the theSingleton is initialized and hence both auxNestedClass and controller could return non-null and true before the initialization (or during) and a partially or uninitialized instance may be returned.

Second, read/write barriers. The JVM isn't actually required to flush everything to main memory until it reaches a write barrier when it exits the critical section. This means that even if your code executed as expected another thread could get a reference to auxNestedClass before the main memory is fully updated and consequently would get bad data.

@Marcelo

What you must realize is the problem is the reodering of writes and a lack of read/write barriers. One thread looks at a reference without synchronization. This reference can be assigned before initializing the instance. Hence while one thread is creating it another thread can freely look at the reference without knowledge or care for whether or not the object has finished initialization.

> Under the memory model in JLS 2nd ed. (In effect for

> 1.2 (I think), 1.3 and 1.4, but not sure about

> 1.4.2), the first thread can execute the assignment

> to auxNested class, and then a second thread is

> allowed to see the non-null value of auxNestedClass

> after it's been set but before the constructor has

> completed.

The memory model was changed with Tiger so anything prior to 1.5, including 1.4.2, is operating on the old memory model in the 2nd Ed and DCL won't work.

> What I said about possibly getting multiple instance

> was incorrect, I think, but I'm not sure.

It is. A second thread can get a reference to an object that's not fully initialized because it's not in a critical section for the first null check and no read/write barriers have been passed. You'd never be able to have a second thread enter the critical section and create a second instance because the second null check would fail.

> Nope, sorry. That won't work. It's still DCL and it's

> still broken. You can still get multiple instances.

> Under the old memory model, you can also get a

> corrupted instance. The new MM might prevent the

> corrupted instance, I'm not sure.

DCL should work fine in the new memory model.

I never have understood the motivation and for that matter I still don't. The JVM implements lazy loading anyway so instantiating in the declaration is effectively a lazy initialization. The only time it's not is if you actually expect to access static members before an instance is ever created which I find highly unusual. Even so the initialization would have to be costly for it to matter. So it's a big to do over some perceived performance need. In most cases it's probably doing harming performance by requiring an unnecessary conditional statement for some perceived benefit of lazy instantiation that was probably already achieved because of lazy loading to begin with. Worst part is I think most of the DCL hoopla is due to the performance hits synchronization caused several years ago which is negligible these days if anything.

If you want lazy instantiation because you expect to use static members before ever creating an instance then use a synchronized getInstance. Otherwise instantiate in the declaration and don't worry about it.

kablaira at 2007-7-20 20:52:46 > top of Java-index,Other Topics,Patterns & OO Design...
# 18

Thank you very much, you all. Especially to kablair, for his/her detailed comments about my code. I've understood what you are saying, and I agree, totally. Yeah, even using a constructor, the solution doesn't solve the problem. Besides, fortunately, we won't have this problem anymore in J2SE 1.5, 1.6, etc, according to you. But unfortunately, I think J2SE 1.4.2 will be used for some years yet (even 1.3 is still being used nowadays!!!), and therefore we have to face this problem yet. The most simple solution is using initialized static fields, as Saish presented to us, and it is a reliable solution.

> First of all, the auxNestedClass can be assigned

> before it is initialized. Hence another thread can

> see auxNestedClass as non-null. You're hoping that

> looking at the boolean variable will solve this, but

> it won't. The controller variable can be assigned

> before the theSingleton is initialized and hence both

> auxNestedClass and controller could return non-null

> and true before the initialization (or during) and a

> partially or uninitialized instance may be returned.

Ok, I was thinking about it, and you are right.

> Second, read/write barriers. The JVM isn't actually

> required to flush everything to main memory until it

> reaches a write barrier when it exits the critical

> section. This means that even if your code executed

> as expected another thread could get a reference to

> auxNestedClass before the main memory is fully

> updated and consequently would get bad data.

I did not understand very well about this point. What does "bad data" mean? Can it cause exception? Might a partial constructed method of the singleton instance be called, and therefore a very serious problem can happen? Might some Error exception be thrown? Is it what "bad data" means?

> What you must realize is the problem is the reodering

> of writes and a lack of read/write barriers. One

> thread looks at a reference without synchronization.

> This reference can be assigned before initializing

> g the instance. Hence while one thread is creating

> it another thread can freely look at the reference

> without knowledge or care for whether or not the

> object has finished initialization.

Ok.

> The memory model was changed with Tiger so anything

> prior to 1.5, including 1.4.2, is operating on the

> old memory model in the 2nd Ed and DCL won't work.

Thank you very much, Sun!!!

> DCL should work fine in the new memory model.

>

> I never have understood the motivation and for that

> matter I still don't. The JVM implements lazy

> loading anyway so instantiating in the declaration is

> effectively a lazy initialization.

Hmmm... you are saying that you don't understand the goal of DCL, do you? I think DCL is clever because if it works well, you won't depend on the entire initialization of the class. You can have some little pieces of initialization in your class, and you will initialize these pieces only when you really need to use them, and you don't need to initialize all of them at the same time. I think it is the main benefit of DCL.

> If you want lazy instantiation because you expect to

> use static members before ever creating an instance

> then use a synchronized getInstance. Otherwise

> instantiate in the declaration and don't worry about

> it.

Yes, I agree. We don't need to worry about it. But, although you are saying that, and although you are saying that DCL should work fine in the new memory model, too, I'd like to comment a little more, just to improve my understanding about synchronization. So, what do you think about my next solution below?

public class Singleton {

private static boolean ready = false;

private static Singleton.NestedClass auxNestedClass = null;

private static Object locker = new Object();

private Singleton() { }

public static Singleton getInstance() {

if (Singleton.ready) {

//don't need to verify anymore...

return auxNestedClass.getSingleton();

} else {

if (auxNestedClass == null) {

synchronized (locker) {

if (auxNestedClass == null)

auxNestedClass = new Singleton.NestedClass();

Singleton.ready = true;

return auxNestedClass.getSingleton();

}

} else if (auxNestedClass.verifier()) {

Singleton.ready = true;

return auxNestedClass.getSingleton();

} else {

return Singleton.getInstance();

}

}

}

private static class NestedClass {

private Singleton.NestedClass.OtherNestedClass otherNestedClass =

new Singleton.NestedClass.OtherNestedClass();

private NestedClass() {

synchronized(otherNestedClass) {

otherNestedClass.theSingleton = new Singleton();

otherNestedClass.controller = true;

}

}

private boolean verifier() {

synchronized(otherNestedClass) {

return otherNestedClass.controller;

}

}

private Singleton getSingleton() {

return otherNestedClass.theSingleton;

}

private class OtherNestedClass {

private Singleton theSingleton = null;

private boolean controller = false;

}

}

}

You can notice that otherNestedClass is being synchronized. Then, now I think the solution I am proposing will solve the problem!!! LOL!!!! Let's see...or maybe I am wrong about how the synchronization of otherNestedClass works, but I think a thread can gain the access of some synchronized instance only after this instance is totally used by other thread. So, I think now my solution solves the problem. Or am I wrong?

Marcelo9a at 2007-7-20 20:52:46 > top of Java-index,Other Topics,Patterns & OO Design...
# 19
I'm not sure (I lost count), but you might have invented triple-checked locking. :^)- Saish
Saisha at 2007-7-20 20:52:46 > top of Java-index,Other Topics,Patterns & OO Design...
# 20
Thanks for straigtening me out, kablair.
jverda at 2007-7-20 20:52:46 > top of Java-index,Other Topics,Patterns & OO Design...
# 21

> DCL should work fine in the new memory model.

I thought I read something in one of these threads a while back (possibly posted by sylviae) that showed that DCL still won't work under the new MM.

I don't remember why, or even if I really did see it. But then, I've never found a reason to use DCL, so it's rather a moot point for me. :-)

jverda at 2007-7-20 20:52:46 > top of Java-index,Other Topics,Patterns & OO Design...
# 22
Dammit man! What about the rest of us wrestling with poor legacy designs?<whimper/>:^)- Saish
Saisha at 2007-7-20 20:52:46 > top of Java-index,Other Topics,Patterns & OO Design...
# 23

Ok, just fixing one line of code, because I think it can make the difference and cause the problem yet:

public class Singleton {

private static boolean ready = false;

private static Singleton.NestedClass auxNestedClass = null;

private static Object locker = new Object();

private Singleton() { }

public static Singleton getInstance() {

if (Singleton.ready) {

//don't need to verify anymore...

return auxNestedClass.getSingleton();

} else {

if (auxNestedClass == null) {

synchronized (locker) {

if (auxNestedClass == null)

auxNestedClass = new Singleton.NestedClass();

//Singleton.ready = true; <-- this line must not exist

return auxNestedClass.getSingleton();

}

} else if (auxNestedClass.verifier()) {

Singleton.ready = true;

return auxNestedClass.getSingleton();

} else {

return Singleton.getInstance();

}

}

}

private static class NestedClass {

private Singleton.NestedClass.OtherNestedClass otherNestedClass =

new Singleton.NestedClass.OtherNestedClass();

private NestedClass() {

synchronized(otherNestedClass) {

otherNestedClass.theSingleton = new Singleton();

otherNestedClass.controller = true;

}

}

private boolean verifier() {

synchronized(otherNestedClass) {

return otherNestedClass.controller;

}

}

private Singleton getSingleton() {

return otherNestedClass.theSingleton;

}

private class OtherNestedClass {

private Singleton theSingleton = null;

private boolean controller = false;

}

}

}

Marcelo9a at 2007-7-20 20:52:46 > top of Java-index,Other Topics,Patterns & OO Design...
# 24

Or probably my version of singleton can be simpler like this:

public class Singleton {

private static boolean ready = false;

private static Singleton theSingleton = null;

private static Object locker = new Object();

private Singleton() { }

public static Singleton getInstance() {

if (Singleton.ready) {

//don't need to verify anymore...

return theSingleton;

} else {

synchronized (locker) {

if (theSingleton == null) {

theSingleton = new Singleton();

return theSingleton;

} else {

Singleton.ready = true;

return Singleton.getInstance();

}

}

}

}

}

I think this version above is equivalent to the other in reply 24.

Marcelo9a at 2007-7-20 20:52:46 > top of Java-index,Other Topics,Patterns & OO Design...
# 25

> I thought I read something in one of these threads a

> while back (possibly posted by sylviae) that showed

> that DCL still won't work under the new MM.

>

> I don't remember why, or even if I really did see it.

> But then, I've never found a reason to use DCL, so

> it's rather a moot point for me. :-)

I'm just going off what Pugh wrote. DCL works if the variable is volatile but using DCL is still a bad idea and is slower than just making use of lazy loading.

> I did not understand very well about this point. What

> does "bad data" mean? Can it cause exception? Might a

> partial constructed method of the singleton instance

> be called, and therefore a very serious problem can

> happen? Might some Error exception be thrown? Is it

> what "bad data" means?

It means that according the JLS there are specific read and write barriers in which a thread's local memory must be written to main memory or read from main memory. Now, understanding that I'm no expert here, what I understand is that entering the synchronized block and exiting the synchronized block would constitute the read and write barriers for this code. The thread creating the object could assign and fully initialize the object and another thread may observe the object before the first thread passes a write barrier. Consequently even if the object is fully initialized the thread that created it may have not flushed everything to main memory yet and the second thread observing it might see corrupted data.

> Hmmm... you are saying that you don't understand the

> goal of DCL, do you? I think DCL is clever because if

> it works well, you won't depend on the entire

> initialization of the class. You can have some little

> pieces of initialization in your class, and you will

> initialize these pieces only when you really need to

> use them, and you don't need to initialize all of

> them at the same time. I think it is the main benefit

> of DCL.

Let me rephrase. I understand why people want the benefits, I don't understand why they're fixated on DCL to achieve it. The cost of synchronization is negligible on modern JVMs and they implement lazy loading already. It's just plain silly. In fact, even in the new memory model you'd need to make the variable volatile which would mean DCL would actually be slower. Pugh suggests a "Initialization On Demand Holder" idiom which leverages the lazy loading:

public final class Singleton {

public Singleton getInstance() {

return SingletonHolder.instance;

}

private static class SingletonHolder {

private static Singleton instance = new Singleton();

}

}

SingletonHolder won't be loaded until it's referenced and consequently we get lazy initialization. It's completely thread-safe and requires no synchronizaton.

kablaira at 2007-7-20 20:52:46 > top of Java-index,Other Topics,Patterns & OO Design...
# 26

> Let me rephrase. I understand why people want the

> benefits, I don't understand why they're fixated on

> DCL to achieve it. The cost of synchronization is

> negligible on modern JVMs and they implement lazy

> loading already. It's just plain silly. In fact,

> even in the new memory model you'd need to make the

> variable volatile which would mean DCL would actually

> be slower. Pugh suggests a "Initialization On

> Demand Holder" idiom which leverages the lazy

> loading:

Yes, people don't need DCL anymore. But, about the cost of synchronization is negligible on modern JVM...I do not know...at first glance I disagree. I think it is always good avoiding synchronization, whenever possible.

Pugh's solution is so simple (who's Pugh?), and so efficient!!! And I was not able to imagine such solution!!! I am depressed now...LOL!!!!

public final class Singleton {

public Singleton getInstance() {

return SingletonHolder.instance;

}

private static class SingletonHolder {

private static Singleton instance = new Singleton();

}

}

> SingletonHolder won't be loaded until it's referenced

> and consequently we get lazy initialization. It's

> completely thread-safe and requires no synchronizaton.

yes, I can see. That's much better than DCL.

Marcelo9a at 2007-7-20 20:52:46 > top of Java-index,Other Topics,Patterns & OO Design...
# 27

> Yes, people don't need DCL anymore. But, about the

> cost of synchronization is negligible on modern

> JVM...I do not know...at first glance I disagree. I

> think it is always good avoiding synchronization,

> whenever possible.

What do you base this on?

I would say don't synchronize unnecessarily, but if the design calls for syncing, then do it. Don't go through all kinds of machinations to avoid it just because you think it might cause a performance hit.

jverda at 2007-7-20 20:52:46 > top of Java-index,Other Topics,Patterns & OO Design...
# 28
That nested class solution is fairly elegant, if you really do have the need to have lazy instantiation and to avoid syncing (which is pretty rare, IME). I've seen that before, but never needed to use it.
jverda at 2007-7-20 20:52:46 > top of Java-index,Other Topics,Patterns & OO Design...
# 29

> Yes, people don't need DCL anymore. But, about the

> cost of synchronization is negligible on modern

> JVM...I do not know...at first glance I disagree. I

> think it is always good avoiding synchronization,

> whenever possible.

Well, to be honest I don't know myself what the performance implications are because I've never tested it or profiled it. Instead I trust in minds greater than mine, such as Henry Wong, when they say that in modern JVMs it's not enough to matter most of the time and shouldn't be worried about until the application is "tuned" or optimized.

Avoiding unnecessary synchronization is a good idea but there's a point at which it crosses over into premature optimization. I subscribe to the notion that we should worry more about code being readable and understandable than whether or not there's a performance concern that may or may not matter.

> Pugh's solution is so simple (who's Pugh?), and so

> efficient!!! And I was not able to imagine such

> solution!!! I am depressed now...LOL!!!!

[url=http://www.cs.umd.edu/~pugh/]Bill Pugh[/url] is a UMD professor who was the driving force and mind behind JSR 133 which is the new memory model.

> That nested class solution is fairly elegant, if you

> really do have the need to have lazy instantiation

> and to avoid syncing (which is pretty rare, IME).

> I've seen that before, but never needed to use it.

Elegant, fast, simple and easy to understand.I wish I'd throught of it.

kablaira at 2007-7-20 20:52:46 > top of Java-index,Other Topics,Patterns & OO Design...
# 30
Great citations MrB! I'll have to check out that JSR? It underpins the new memory model in Mustang?- Saish
Saisha at 2007-7-20 20:52:54 > top of Java-index,Other Topics,Patterns & OO Design...
# 31

> > Yes, people don't need DCL anymore. But, about the

> > cost of synchronization is negligible on modern

> > JVM...I do not know...at first glance I disagree.

> I

> > think it is always good avoiding synchronization,

> > whenever possible.

>

>

> What do you base this on?

>

> I would say don't synchronize unnecessarily, but if

> the design calls for syncing, then do it. Don't go

> through all kinds of machinations to avoid it just

> because you think it might cause a performance hit.

When I say "it is always good avoiding synchronization, whenever possible", I mean: don't use synchronization if you don't need it. To clarify more giving an example, don't use a synchronized arraylist if you are sure that you can safely use a simple unsynchronized arraylist. This idea is what I mean. Of course, when you really have to use synchronization, then use it.

In the particular case of singleton, I think we can avoid synchronization, using Pugh's solution. I liked this solution. Why cannot we use Pugh's solution, instead of the classic singleton pattern with a synchronized method? We can't just because Pugh's solution is "ugly"? I don't think it is ugly. For me it is very elegant implementation of singleton, and it is reliable.

Marcelo9a at 2007-7-20 20:52:54 > top of Java-index,Other Topics,Patterns & OO Design...
# 32

> Great citations MrB! I'll have to check out that

> JSR? It underpins the new memory model in Mustang?

>

> - Saish

It's the JSR for the new memory model but it's target was Tiger (1.5) and it is part of the JLS 3rd Ed.I don't know what version Mustang is off the top of my head (1.6?).

kablaira at 2007-7-20 20:52:54 > top of Java-index,Other Topics,Patterns & OO Design...
# 33

> this solution. Why cannot we use Pugh's solution,

> instead of the classic singleton pattern with a

> synchronized method? We can't just because Pugh's

> solution is "ugly"? I don't think it is ugly.

Nor do I. I think either one (Pugh or syncing) is fine.

When I was talking about making things ugly, I just meant in general, and also some of the convoluted attempts at DCL that I've seen here and in other places.

jverda at 2007-7-20 20:52:54 > top of Java-index,Other Topics,Patterns & OO Design...
# 34
> [url= http://www.cs.umd.edu/~pugh/]Bill Pugh[/url] is> a UMD professor who was the driving force and mind> behind JSR 133 which is the new memory model. Hey, kablair! This site is very cool!!! Thank you!!! ;-)
Marcelo9a at 2007-7-20 20:52:54 > top of Java-index,Other Topics,Patterns & OO Design...