Java singleton class and multi-threading

Question

Folks- I have a helper class, the task of which is to build some messages based on the parameters. The class by itself does not have any private data (other than the instance ofcourse).

public class RequestBuilder {
    private static RequestBuilder instance = new RequestBuilder();

    private RequestBuilder() {}

    public static RequestBuilder getInstance() {
        return instance;
    }

    public SetRequest buildSetRequest(Path prefix,
                                      Path path,
                                      ConfigEntity configEntity,
                                      Any protoAnyData) {
        .....
        .....
        return setRequest;
    }

    public GetRequest buildGetRequest(Path prefix,
                                      Path Path,
                                      RetrieveRequestEntity retrieveRequestEntity,
                                      Encoding encoding) {
        .....
        .....
        return getRequest;
    }
}

I understand singleton classes are not multithreading friendly. In this case, what happens when 2 threads try to execute buildSetRequest() at the same time?

Thanks for your time.

EDIT: Based on my need, and as suggested by @BoristheSpide in the comments below, I'm going to make this class as a utility class with the following changes: 1. Make it final. 2. Make methods static. 3. Remove all singleton references.

public final class RequestBuilder {

    private RequestBuilder() {}

    public static SetRequest buildSetRequest(Path prefix,
                                      Path path,
                                      ConfigEntity configEntity,
                                      Any protoAnyData) {
        .....
        .....
        return setRequest;
    }

    public static GetRequest buildGetRequest(Path prefix,
                                      Path Path,
                                      RetrieveRequestEntity retrieveRequestEntity,
                                      Encoding encoding) {
        .....
        .....
        return getRequest;
    }
}

I'm leaving the original code as is, because it is still valid and gives context to the comments and answers to this question.

If there is no shared state - i.e. no fields - then it's fine. — Boris the Spider
– Boris the Spider, Commented Feb 28, 2019 at 19:30
... although that does beg the question of why you need a singleton at all. Just make it a utility class and be done with it. — Boris the Spider
– Boris the Spider, Commented Feb 28, 2019 at 19:41
Well what does buildSetRequest exactly? If that call modifies the configEntity argument; and the two calling threads are using the same ConfigEntity instance.. Then you can have problems (since you have shared mutable state) — Daniele
– Daniele, Commented Feb 28, 2019 at 21:07

tastydb · Accepted Answer · 2019-03-01 22:01:26Z

1

In this case, not too much, because your constructor is empty (what others mention as no shared state). Problems start when you have multiple private instance variables that you have to initialize. In that case, you need some protection like the doble-check:

private static volatile RequestBuilder instance;

private RequestBuilder() {}

public static RequestBuilder getInstance() {
    if (instance == null) {
        synchronized (RequestBuilder.class) {
            if (instance == null) {
                instance = new RequestBuilder();
            }
        }
    }
    return instance;
}

The reason is that a thread can be suspended at any time. If the current thread constructing the instance is suspended, and another one comes, there can be half initialized instance variables and the object can end up in a corrupted state.

EDIT: About buildSetRequest()

The code is inside a method, if this method create it's own instances itself or works with thread safe classes, there won't be any problem.

edited Mar 1, 2019 at 22:01

answered Feb 28, 2019 at 19:24

tastydb

5421 gold badge3 silver badges16 bronze badges

Sign up to request clarification or add additional context in comments.

9 Comments

Boris the Spider Over a year ago

This is just the difference between lazy or eager initialisation. Please read the rules around class initialisers - the code the OP has is perfectly safe from a construction standpoint.

tastydb Over a year ago

@BoristheSpider Thanks for the negative vote. Yes, but the OP mixes the fact that's a singleton class with the call to buildSetRequest() and I didn't know how to focus the answer. Thanks for your feedback and feel free to expand my explanation.

Boris the Spider Over a year ago

Sorry, this is not about the focus of your answer. This is about the entire first part of your answer being incorrect. There is no way, given the Java language specification, that a half initialised instance can leak. Your suggestion adds spurious complexity to spurious "facts" about concurrency.

tastydb Over a year ago

@BoristheSpider Whatever, but what I answered is the correct way to go. Check this and you will see that, effectively, Java memory model allows the publication of partially initialized objects: https://stackoverflow.com/questions/45857765/partial-constructed-objects-in-the-java-memory-model. Again, you can always answer the OP's question too.

Boris the Spider Over a year ago

That's a completely different example. You are suggesting that a class initialiser is not protected by the JVM. The constructor call is protected by the class initialiser. I don't know what more to say - this answer is fundamentally incorrect and spreads dangerous misinformation about how Java works. I ask you as a professional to delete it.

|

Collectives™ on Stack Overflow

Java singleton class and multi-threading

1 Answer 1

9 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

9 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related