How to make builder pattern thread safe in the multithreading environment?

Question

I am working on a project in which I need to have synchronous and asynchronous method of my java client. Some customer will call synchronous and some customer will call asynchronous method of my java client depending on there requirement.

Below is my java client which has synchronous and asynchronous methods -

public class TestingClient implements IClient {

    private ExecutorService service = Executors.newFixedThreadPool(10);
    private RestTemplate restTemplate = new RestTemplate();

    // for synchronous
    @Override
    public String executeSync(ClientKey keys) {

        String response = null;
        try {

            Future<String> handle = executeAsync(keys);
            response = handle.get(keys.getTimeout(), TimeUnit.MILLISECONDS);
        } catch (TimeoutException e) {

        } catch (Exception e) {

        }

        return response;
    }

    // for asynchronous
    @Override
    public Future<String> executeAsync(ClientKey keys) {

        Future<String> future = null;

        try {
            ClientTask ClientTask = new ClientTask(keys, restTemplate);
            future = service.submit(ClientTask);
        } catch (Exception ex) {

        }

        return future;
    }
}

And now below is my ClientTask class which implements Callable interface and I am passing around the dependency using DI pattern in the ClientTask class. In the call method, I am just making a URL basis on machineIPAddress and using the ClientKeys which is passed to ClientTask class and then hit the server using RestTemplate and get the response back -

class ClientTask implements Callable<String> {

    private ClientKey cKeys;
    private RestTemplate restTemplate;

    public ClientTask(ClientKey cKeys, RestTemplate restTemplate) {
        this.restTemplate = restTemplate;
        this.cKeys = cKeys;
    }

    @Override
    public String call() throws Exception {

        // .. some code here
        String url = generateURL("machineIPAddress");           
        String response = restTemplate.getForObject(url, String.class);

        return response;
    }

    // is this method thread safe and the way I am using `cKeys` variable here is also thread safe?
    private String generateURL(final String hostIPAdress) throws Exception {
        StringBuffer url = new StringBuffer();
        url.append("http://" + hostIPAdress + ":8087/user?user_id=" + cKeys.getUserId() + "&client_id="
            + cKeys.getClientId());

        final Map<String, String> paramMap = cKeys.getParameterMap();
        Set<Entry<String, String>> params = paramMap.entrySet();
        for (Entry<String, String> e : params) {
            url.append("&" + e.getKey());
            url.append("=" + e.getValue());
        }

        return url.toString();
    }
}

And below is my ClientKey class using Builder patter which customer will use to make the input parameters to pass to the TestingClient -

public final class ClientKey {

    private final long userId;
    private final int clientId;
    private final long timeout;
    private final boolean testFlag;
    private final Map<String, String> parameterMap;

    private ClientKey(Builder builder) {
    this.userId = builder.userId;
    this.clientId = builder.clientId;
    this.remoteFlag = builder.remoteFlag;
    this.testFlag = builder.testFlag;
    this.parameterMap = builder.parameterMap;
    this.timeout = builder.timeout;
    }

    public static class Builder {
    protected final long userId;
    protected final int clientId;
    protected long timeout = 200L;
    protected boolean remoteFlag = false;
    protected boolean testFlag = true;
    protected Map<String, String> parameterMap;

    public Builder(long userId, int clientId) {
        this.userId = userId;
        this.clientId = clientId;
    }

    public Builder parameterMap(Map<String, String> parameterMap) {
        this.parameterMap = parameterMap;
        return this;
    }

    public Builder remoteFlag(boolean remoteFlag) {
        this.remoteFlag = remoteFlag;
        return this;
    }

    public Builder testFlag(boolean testFlag) {
        this.testFlag = testFlag;
        return this;
    }

    public Builder addTimeout(long timeout) {
        this.timeout = timeout;
        return this;
    }

    public ClientKey build() {
        return new ClientKey(this);
    }
    }

    public long getUserId() {
    return userId;
    }

    public int getClientId() {
    return clientId;
    }

    public long getTimeout() {
    return timeout;
    }

    public Map<String, String> getParameterMap() {
    return parameterMap;

    public boolean istestFlag() {
    return testFlag;
    }
}

Is my above code thread safe as I am using ClientKey variables in ClientTask class in multithreaded environment so not sure what will happen if another thread tries to make ClientKey variable while making a call to TestingClient synchronous method -

Because customer will be making a call to us with the use of below code and they can call us from there Multithreaded application as well -

IClient testClient = ClientFactory.getInstance();

Map<String, String> testMap = new LinkedHashMap<String, String>();
testMap.put("hello", "world");

ClientKey keys = new ClientKey.Builder(12345L, 200).addTimeout(2000L).parameterMap(testMap).build();

String response = testClient.executeSync(keys);

So just trying to understand whether my above code will be thread safe or not as they can pass multiple values to my TestingClient class from multiple threads. I am having a feeling that my ClientKey class is not thread safe because of parameterMap but not sure.

And also do I need StringBuffer here or StringBuilder will be fine as StringBuilder is faster than StringBuffer because it's not synchronized.

Can anyone help me with this?

Answer 1

The parameter ClientKey keys is given, so I assume is always different.

I don't see any synchronization issues with your code, I'll explain:

ClientTask ClientTask = new ClientTask(keys, restTemplate);
future = service.submit(ClientTask);

• Creating a ClientTask object from inside the method, which is not shared among threads.
• Using service.submit, whih returns a Future object
• The ClientTask object read the keys only inside the method generateURL, but, as I said before, the ClientKeys object is given as a parameter, so you are good as long as this object is not being shared.

In summary, the thread-safeness of your code depends on ExecutorService and Future being thread safe.

Update: Clarification for as long as this object is not being shared

ClientKeys keys;
add keys to @keys
.. code
executeAsync(.., keys)
... code
add keys to @keys
add keys to @keys
executeAsync(.., keys)
executeAsync(.., keys)
add keys to @keys
... code
add keys to @keys
executeAsync(.., keys)

This is (more and less) what I meant is sharing. keys is being used in several threads due to the calls to executeAsync(). In this case, some threads are reading keys, and others are writing data to it, causing whats is ussualy called a race condition.

Update 2: The StringBuffer object is local to (aka is in the scope of) generateURL, there's no need to synchronize it.