• Post Reply Bookmark Topic Watch Topic
  • New Topic

Singleton  RSS feed

 
Markus Schmider
Ranch Hand
Posts: 149
3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have a problem with Singletons (in an JSE application)



I thought that the above code should guarantee only on single instance in the jvm. But calling it from an EntityListener I get many instances until the application crashes because the database connections are used up.
What's wrong?
 
Markus Schmider
Ranch Hand
Posts: 149
3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
With this implementation the problem dissapears

 
Campbell Ritchie
Marshal
Posts: 56599
172
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Of course this is the correct way to create a SingletonYou can add fields and methods to that instance. It's all in the Java Tutorials.
 
Chan Ag
Rancher
Posts: 1090
14
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
What Campbell said++.

That said, if I ignore the reflection attack possibility, and assume that the only way the variable instance is being accessed is through the methods shown in the code, and that the only instance of PersistenceService that is declared in the class PersistenceService is instance, I don't understand how the first code can create more than a single instance of PersistenceService.

The synchronized method getInstance should create a memory barrier and prevent multiple instances of PersistenceService from being created.

Markus, you've asked to mark this topic as resolved. So have you found out why the first case can create more than one instance?
As for me, I still don't have that answer.


 
Stevens Miller
Bartender
Posts: 1445
30
C++ Java Netbeans IDE Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I agree with Campbell and Chan. There's a simpler alternative, but it also seems like your first version ought to work.

As I understand synchronized, it should guarantee memory coherence, making sure different threads aren't hiding their assignments to instance from each other by caching their copies. Regardless, I'd be tempted to make it a volatile in your first version, just to see if that makes any difference.
 
Chan Ag
Rancher
Posts: 1090
14
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stevens Miller wrote:Regardless, I'd be tempted to make it a volatile in your first version, just to see if that makes any difference.


That's what I thought also. But it shouldn't make a difference. If it does, that'll be a thing I learned today ( for good ).
 
Paul Clapham
Sheriff
Posts: 22841
43
Eclipse IDE Firefox Browser MySQL Database
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In the first piece of code, if Persistence.createEntityManagerFactory("Wasserrecht") calls PersistenceService.getInstance() then synchronizing the latter method doesn't prevent simultaneous access to the getInstance() method.
 
Jaikiran Pai
Sheriff
Posts: 10447
227
IntelliJ IDE Ubuntu
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I think the reason why the first one fails has more to do with the code in thw constructor and its use in that static method than anything to do with multi threading. The important bit is that Markus states, the calls originate from a JPA entity listener.

Consider this, thread A somehow endsup calling that static synchronized getInstance method for first time. instance member is null so the code calls the constructor which starts creating entity manager factory which then triggers perhaps the onload or some such entity listener which then again calls that static synchronized getInstance method. All of this on same thread A. Since its the same thread holding the lock, it allowed to enter the method and it then sees that instance member is null (because the previous constructor callhas still not ffinished), so it calls the constructor again and the whole thing repeats and goes into a cycle and since the entity manager factory creation leads to DB connection creation and usage, this kind of cycle blows up the connection count.

At least that's the only explanation I can come up with for this code.

Edit: This is why I hate using mobile device for typing. When I started typing my reply Paul's reply wasn't yet in and by the time I finished typing I see Paul has replied and the time difference between our posts is 13 minutes! Can't believe I spent that long to type it!

 
Stevens Miller
Bartender
Posts: 1445
30
C++ Java Netbeans IDE Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paul Clapham wrote:In the first piece of code, if Persistence.createEntityManagerFactory("Wasserrecht") calls PersistenceService.getInstance() then synchronizing the latter method doesn't prevent simultaneous access to the getInstance() method.

I've been thinking about this for a few minutes and can't see why not. I'm sure it's something basic, but I'm just not seeing it. Can you say a bit more?
 
Paul Clapham
Sheriff
Posts: 22841
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I don't know why people have such an attachment to the idea of lazy initialization of a singleton object anyway. If there was a chance that the singleton would never be used, and if there was a significant cost in creating the singleton, then yeah, okay. But those conditions almost never apply. Like Campbell says, just use the enum.
 
Paul Clapham
Sheriff
Posts: 22841
43
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stevens Miller wrote:
Paul Clapham wrote:In the first piece of code, if Persistence.createEntityManagerFactory("Wasserrecht") calls PersistenceService.getInstance() then synchronizing the latter method doesn't prevent simultaneous access to the getInstance() method.

I've been thinking about this for a few minutes and can't see why not. I'm sure it's something basic, but I'm just not seeing it. Can you say a bit more?


Jaikiran did just say more. Read his post for a description of how that might actually happen.
 
Stevens Miller
Bartender
Posts: 1445
30
C++ Java Netbeans IDE Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paul Clapham wrote:
Stevens Miller wrote:
Paul Clapham wrote:In the first piece of code, if Persistence.createEntityManagerFactory("Wasserrecht") calls PersistenceService.getInstance() then synchronizing the latter method doesn't prevent simultaneous access to the getInstance() method.

I've been thinking about this for a few minutes and can't see why not. I'm sure it's something basic, but I'm just not seeing it. Can you say a bit more?


Jaikiran did just say more. Read his post for a description of how that might actually happen.


I don't mean to be an argumentative ass (it just comes to me naturally, sometimes ;-) ), but I think I understand Jaikiran's analysis (very impressive, btw), and it seems to me that he is not showing how access might be simultaneous. His explanation shows how getInstance might be called recursively. By "simultaneous access," I take you to mean that getInstance could be entered concurrently by more than one thread, which I would think was impossible for a synchronized method. (If, by "simultaneous access," you meant, "entered more than once on the same thread," of course, then I think I understand you now.)
 
Paul Clapham
Sheriff
Posts: 22841
43
Eclipse IDE Firefox Browser MySQL Database
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The "synchronized" attribute prevents a method from being accessed by more than one thread at a time. In other words once Thread A enters the method, Thread B can't enter the method until Thread A exits the method. But a recursive call from Thread A can enter the method even though the earlier call from Thread A hasn't exited the method yet. That appears to be why "synchronized" isn't preventing multiple objects from being created.
 
Stevens Miller
Bartender
Posts: 1445
30
C++ Java Netbeans IDE Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paul Clapham wrote:The "synchronized" attribute prevents a method from being accessed by more than one thread at a time. In other words once Thread A enters the method, Thread B can't enter the method until Thread A exits the method. But a recursive call from Thread A can enter the method even though the earlier call from Thread A hasn't exited the method yet. That appears to be why "synchronized" isn't preventing multiple objects from being created.

Got it. I'm just trying to be certain I get my terms right (or else Winston will ding me again). By "simultaneous access," did you mean to include single-threaded reentrance?
 
Stevens Miller
Bartender
Posts: 1445
30
C++ Java Netbeans IDE Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Paul Clapham wrote:I don't know why people have such an attachment to the idea of lazy initialization of a singleton object anyway. If there was a chance that the singleton would never be used, and if there was a significant cost in creating the singleton, then yeah, okay. But those conditions almost never apply. Like Campbell says, just use the enum.

Agreed. Elegance for the sake of elegance is often just another bug waiting to be found. Double-checked locking, anyone?
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!