• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Making Singleton Thread safe

 
Greenhorn
Posts: 6
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi, I have a class as below -



When I ran PMD for this class it showed NonThreadSafeSingleton Error. I modified the getList() method with the double null check synchronised block as shown in the below code but still PMD shows NonThreadSafeSingleton Error. I am not able to figure it out why PMD still shows this error. Any suggestion how can I get rid of this error?

 
Marshal
Posts: 28193
95
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Get rid of the unnecessary complications. For example, if you really need lazy initialization, then just synchronize the getList() method. But you don't really need that, so don't do it. Here's a simpler version:



Or even simpler, write the class as an "enum" with a single value.
 
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Rohit Kejriwal wrote:it showed NonThreadSafeSingleton Error.



Just FYI, that's a warning, not an error.

A warning doesn't mean you've done something illegal in the language, and it won't prevent a .class file from being generated. It just means you've done something that could lead to unexpected or undesirable behavior.

An error means you've violated the rules of the language, and prevents a .class file from being produced.
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Paul Clapham wrote:Get rid of the unnecessary complications. For example, if you really need lazy initialization, then just synchronize the getList() method. But you don't really need that, so don't do it. Here's a simpler version:



Or even simpler, write the class as an "enum" with a single value.



And note that this only makes the Singleton object threadsafe. Since the ArrayList is modifiable, it's still susceptible to thread-unsafe usage. And of course, even if you made the list unmodifiable, if its elements are mutable, then they could be used a thread-unsafe manner.
 
Master Rancher
Posts: 4806
72
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I agree with everything Paul and Jeff have said. I would add that the whole idea of double-checked locking was completely broken for a long time before Java 5 came out; since then, code such as you've shown works only if the variable (myList, in this case) is declared volatile. Adding that would make the double-checked locking "thread-safe", at least as far as accessing the list itself is concerned, though it would not stop it from being needlessly overcomplicated as well as unsafe since the list is still mutable.
 
Marshal
Posts: 79177
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
You can always convert it to an unmodifiable list with the method of the Collections class. But that still does not immunise the elements of the List against concurrency problems.
 
With a little knowledge, a cast iron skillet is non-stick and lasts a lifetime.
reply
    Bookmark Topic Watch Topic
  • New Topic