File APIs for Java Developers
Manipulate DOC, XLS, PPT, PDF and many others from your application.
http://aspose.com/file-tools
The moose likes Developer Certification (SCJD/OCMJD) and the fly likes Singleton initialization (exceptions, thread-safety) Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Android Security Essentials Live Lessons this week in the Android forum!
JavaRanch » Java Forums » Certification » Developer Certification (SCJD/OCMJD)
Bookmark "Singleton initialization (exceptions, thread-safety)" Watch "Singleton initialization (exceptions, thread-safety)" New topic
Author

Singleton initialization (exceptions, thread-safety)

Piotr Nowicki
Ranch Hand

Joined: Jul 13, 2010
Posts: 610

Howdy Ranchers!

I am working on Data class as a singleton pattern. Firstly I created a thread-safe singleton by initializing the instance as a class field like this (not relevant parts were cut):



But when the Data class is creating, it reads underlying database file. I would like not to catch exceptions thrown when reading the file, but rather bubble them up to the business logic layer and then send to client to inform that sever error occured. I cannot throw exceptions from the constructor, because I create an object of the class as a field. The only thing I could came up with is to move the read code into getInstance() method which can throw exceptions like this:



My concern is thread-safety of this solution. There is only one instance of the object (it is created when loading class) so it's fine, but two threads can access the same object's method (getInstance()) and do something like:

T1: instance.recordCache == null -> TRUE
T2: instance.recordCache == null -> TRUE
T1: instance.recordCache = instance.dataAccessor.read();
T2: instance.recordCache = instance.dataAccessor.read();

And I've ended with loading the records twice. It is not a big problem, as it won't corrupt any data, but it is an overhead which I don't need.

I can synchronize whole getInstance() method, but won't this impact a performance a lot?

What do you think about it?
How would you solve a problem of throwing an exceptions from singleton?
Do you see any design flaws in my approach?
Do you find it more coherent to create whole object in getInstance() method instead of instantiasing it as a field AND then doing some reading operations in getInstance() method?

Thanks in advance Guys!

-----

EDIT: One of the possibilities is to throw a Runtime Exception from the static initializer block, but it will made me to wrap my checked exception into runtime exception and then (in business logic layer) back to checked one for the GUI... This smells, don't you think?


OCP Java SE 6 Programmer, OCM Java SE 6 Developer, OCE Java EE 6 JSPSD, OCE Java EE 6 EJBD, OCE Java EE 6 JPAD, Spring 3.0 Core Professional.
David Byron
Rancher

Joined: Jan 20, 2009
Posts: 171

You have considered swallowing the exceptions in the private constructor and throwing them from the getInstance().
Have you tried throwing your exceptions from the private constructor?

I cannot throw exceptions from the constructor, because I create an object of the class as a field

Are you sure about that?


SCJD 6, OCPJP7, Baroque Potion, G+
Piotr Nowicki
Ranch Hand

Joined: Jul 13, 2010
Posts: 610

Hi David! Well... I'm pretty sure I cannot throw checked exceptions from the private constructor if this class object is instantiated as a class member... the initializing block is no use, as I can only catch exceptions within it, not rethrow (no place to catch them).

I don't think I'm considering swallowing exceptions in the constructor, but after rethinking, it might be cleaner to move whole read operation and object instantiasion to the getInstance() method... do you think otherwise?

Is this approach design-corrent?
Arkin Yetis
Greenhorn

Joined: Oct 13, 2010
Posts: 4
Hi Pedro,

Check this out: Double-checked locking

It basically boils down to: with JDK 5 and above your thread-safety concern is addressed with a few modifications. Before then it seems to be broken... Really interesting article, if you are into nitty gritty details.
David Byron
Rancher

Joined: Jan 20, 2009
Posts: 171

Pedro Kowalski wrote:Hi David! Well... I'm pretty sure I cannot throw checked exceptions from the private constructor if this class object is instantiated as a class member...


Ah-- but if you throw an exception during construction of the class, then it's never fully instantiated and therefore never reaches the point of being a class member.

I don't think I'm considering swallowing exceptions in the constructor, but after rethinking, it might be cleaner to move whole read operation and object instantiasion to the getInstance() method... do you think otherwise?


I think reading from your file and manipulating a cache every time someone gets an instance of your singleton is a questionable design choice.

I threw an exception from the singleton constructor in my project, which is why I'm pretty sure it works ;) . In particular cases (such as this one) it's wise to do so. After all, having legitimate access to the data source is a precondition of the Data class's existence. So aborting its construction when that precondition fails makes sense.
Piotr Nowicki
Ranch Hand

Joined: Jul 13, 2010
Posts: 610

David, I totally agree that if there is something wrong with the dataclass it should not be constructed. But then again, I'm not 100% sure if I understand you correctly.

I'll try to rephrase it and show some code to help me understand this.

Let the StorageException be a checked exception.



This code is throwing an exception from the constructor and in getInstance it rethrows them further. This is what I want to get, as I can inform client that during data class initialization, there were some problems.

But this solution implies thread-unsafety, because the getInstance() method should be:
- synchronized (simple, easy to read but a big hit for the multithreading),
- do double-checked locking (thanks a lot Arkin for this article!),
- do... nothing, as possible access for multiple threads do not break the overall functionality - just adds some overhead ONCE (if the instance of Data class is not instantiated)

The thread-safe version I like (clean, without overhead) is this one with class instance as a field initialized while loading the class:



The latter is what I mean by
I cannot throw checked exceptions from the private constructor if this class object is instantiated as a class member...


The above will not compile, as there is no way I can put a try/catch to a class field or more important - thrown the exception above (which is what I want to achieve to be able to inform client that the database initialization didn't success).
David Byron
Rancher

Joined: Jan 20, 2009
Posts: 171

My apology; I misunderstood what you were asking. You're right that one cannot throw exceptions while instantiating the class member as it's declared.

If I now read you correctly, you're worried about a multithreading debacle during the construction of your singleton-- specifically, that concurrent clients could both find your 'instance == null' satisfied.

My implementation looks approximately like your first example, though our fa├žades are different. I regarded that particular concurrency problem as so unlikely that I didn't code defensively against it. Whether you preempt it or not, document your choice in your choices.txt.
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5147
    
  12

I had to deal with the same issue. This is my solution:
  • added a "initialize" method (and a "destroy" one) to my own interface
  • these methods are marked synchronized
  • the initialize-method has to be called before any method can be called on the single Data instance


  • This solution is completely in line with the approach of my application: KISS


    SCJA, SCJP (1.4 | 5.0 | 6.0), SCJD
    http://www.javaroe.be/
    Piotr Nowicki
    Ranch Hand

    Joined: Jul 13, 2010
    Posts: 610

    Just when I sit in the front of the monitor this morning, to write that I've redesigned my Data class I've seen you wrote basicaly exact same thing Roel :-)))

    I've added

    and

    and left the singleton creation as is was originally (clean and thread-safe):


    I do check if the record cache and data accessor are set (so basicaly, if the start(String) method was fired). If not - IllegalStateException will be thrown.

    I found that this approach is also more intuitive when managing the server startup, when it invokes Data.getInstance().start() rather than Data.getInstance(), as the former one is, in my opinion, more explainatory for future developers.

    Thanks Guys for your time!
    Roel De Nijs
    Bartender

    Joined: Jul 19, 2004
    Posts: 5147
        
      12

    Just one small remark about your start-method

    What if the database file is replaced with a rdbms and you have to provide an url, username and password to "start" the Data class?
    Piotr Nowicki
    Ranch Hand

    Joined: Jul 13, 2010
    Posts: 610

    I've documented it in javadoc - you should overload the start method (which then should take i.e. DB host, username and password).

    Of course start and stop are synchronized and returns void (which isn't shown in the snipped above)
    Roel De Nijs
    Bartender

    Joined: Jul 19, 2004
    Posts: 5147
        
      12

    What about this approach?


    You don't have to overload your start-method. You can pass any value to the Data class depending on the underlying database implementation. Your interface stays generic (no indication you are using a database file as database),...
    No need to say my start-method looks exactly like this
    Piotr Nowicki
    Ranch Hand

    Joined: Jul 13, 2010
    Posts: 610

    That's an interesting idea, which made me to rethink my approach.

    When I develop an application I feel unhappy when see the Object type passed as an argument, as at the first glance it makes me unsure what this Object should really be; that's why I'm trying to reduce it's usage when possible (BTW: I don't know if this is the right choice, but that's what I feel it should be - maybe I'm wrong.)

    Perhaps I will come with something between Your approach and mine.

    As the start() method invokes underlying data access manager I can pass this interface as an argument.

    To be more precise - I've got
    DataAccessManager interface which defines what read(...) and save(...) operations are (idea basing on record cache).

    It's implementer is FileAccessManager - for accessing flat-files database which was provided by me, as it's the aim of the assignment. If someone would like to enhance the application for RBDMS usage, he should implement the DataAccessManager in, let's say RDBMSAccessManager.

    I could modify the method to be


    I think this will make it more easy than overloading and a bit more strict about what can be passed as an argument.

    What do you think Roel?
    Roel De Nijs
    Bartender

    Joined: Jul 19, 2004
    Posts: 5147
        
      12

    Seems ok to me.

    I don't see anything wrong with having a parameter of type Object in 1 method (if of course all your methods have nothing but Object parameters, you maybe have to rethink your approach ). Of course anything can be passed to the method, but if you have flawless parameter validation (which I have) and good javadoc comments, little can go wrong.
    As a side note: I also don't have a seperate DataAccessManager like in your approach, just the Data class, so that's why I opted for this approach.

    The most important thing is that you should not have any implementation details in an interface, and dbPath is such an implementation detail. How you solve it, is not that important. You should also not throw for example a FileNotFoundException, but instead a DBException (or something similar).
    Piotr Nowicki
    Ranch Hand

    Joined: Jul 13, 2010
    Posts: 610

    My interface is throwing a StorageException which wraps the IOException in case of Files and perhaps should wrap SQLException when RDBMS would be used.

    Ps. I just wanted to say that if I'm a user of method(Object) I am a bit afraid what is it purpose - I just like to be more strict if it comes to parameters. Of course it depends on overall approach (as in this case) and I think is more about the individual style, which is a topic for separate discussion.

    Thanks for your opinion Roel

    Cheers!
    Roel De Nijs
    Bartender

    Joined: Jul 19, 2004
    Posts: 5147
        
      12

    Pedro Kowalski wrote:that if I'm a user of method(Object) I am a bit afraid what is it purpose

    That's why you should have clear javadoc comments (and so you are a bit afraid of Object's equals method )
    Piotr Nowicki
    Ranch Hand

    Joined: Jul 13, 2010
    Posts: 610

    Roel, it's just that I prefer to be informed about the object type by the method signature. Of course you can inform the client about how to use your method, be very precise about casting, type checking, document it, etc.

    The equals() method is dictated by the design and purpose - which is fine for me.

    It doesn't change the fact that I personally prefer as intuitive and well precised arguments for the methods as possible. It just defines me the well precised contract I have to obey. If defining such contract is impossible or it is the matter of design, the java.lang.Object will be my last resort - as simple as that :-)

    Cheers and thanks for your opinion!
     
    It is sorta covered in the JavaRanch Style Guide.
     
    subject: Singleton initialization (exceptions, thread-safety)
     
    Similar Threads
    Why constuctor can't be marked as final?
    Singleton
    Singleton Class with Threads
    Singleton class + Lazy loading
    Singleton in multithreads