Search code examples
javamultithreadingconcurrencythread-safetysaaj

Creating one non-thread-safe object per thread and using happens-before guarantee


I want to use SOAPConnectionFactory and MessageFactory classes from SAAJ with multiple threads, but it turns out that I can't assume they are thread-safe. Some related posts:

Here is an interesting little proof that it can be thread safe: http://svn.apache.org/repos/asf/axis/axis2/java/core/tags/v1.5.6/modules/saaj/src/org/apache/axis2/saaj/SOAPConnectionImpl.java it is said

Although thread safety is not explicitly required by the SAAJ specs, it appears that the SOAPConnection in Sun's reference implementation is thread safe.

But still I don't think it's enough proof to treat SAAJ classes as thread-safe.

So my question: is the idiom below correct? I create exactly one SOAPConnection and MessageFactory object using the possibly non-thread safe factories inside the main thread and then safely publish those object to an executor task using the happens-before guarantee of the CompletionService interface. I also use this happens-before guarantee to extract the result HashMap object.

Basically I just want to verify the sanity of my reasoning.

public static void main(String args[]) throws Exception {
    ExecutorService executorService = Executors.newFixedThreadPool(10);
    CompletionService<Map<String, String>> completionService = new ExecutorCompletionService<>(executorService);

    //submitting 100 tasks
    for (int i = 0; i < 100; i++) {
        // there is no docs on if these classes are thread-safe or not, so creating them before submitting to the
        // external thread. This seems to be safe, because we are relying on the happens-before guarantees of the
        // CompletionService.
        SOAPConnectionFactory soapConnectionFactory = SOAPConnectionFactory.newInstance();
        SOAPConnection soapConnection = soapConnectionFactory.createConnection();
        MessageFactory messageFactory = MessageFactory.newInstance();
        int number = i;// we can't just use i, because it's not effectively final within the task below
        completionService.submit(() -> {
            // using messageFactory here!
            SOAPMessage request = createSOAPRequest(messageFactory, number);
            // using soapConnection here!
            SOAPMessage soapResponse = soapConnection.call(request, "example.com");
            soapConnection.close();
            ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
            soapResponse.writeTo(outputStream);
            // HashMap is not thread-safe on its own, but we'll use the happens-before guarantee. See f.get() below.
            Map<String, String> result = new HashMap<>();
            result.put("soapResponse", new String(outputStream.toByteArray()));
            return result;

        });
    }

    // printing the responses as they arrive
    for (int i = 0; i < 100; i++) {
        Future<Map<String, String>> f = completionService.take();
        Map<String, String> result = f.get();
        System.out.println(result.get("soapResponse"));
    }

    executorService.shutdown();
}

/**
 * Thread-safe static method
 */
private static SOAPMessage createSOAPRequest(MessageFactory messageFactory, int number) throws Exception {
    SOAPMessage soapMessage = messageFactory.createMessage();
    SOAPPart soapPart = soapMessage.getSOAPPart();

    String serverURI = "example.com";

    SOAPEnvelope envelope = soapPart.getEnvelope();
    envelope.addNamespaceDeclaration("example", serverURI);

    SOAPBody soapBody = envelope.getBody();
    SOAPElement soapBodyElem = soapBody.addChildElement("number", "example");
    soapBodyElem.addTextNode(String.valueOf(number));

    soapMessage.saveChanges();

    return soapMessage;
}

Solution

  • Yes, your reasoning about the CompletionService is correct -- .submit() ensures that the task lambda will see the complete objects, and .take() ensures that the main thread will only see fully formed responses.

    Generally, however, you don't need to do this. static factory methods should always be thread-safe, because there is no way to ensure that they are not in use in some other thread without global knowledge of the whole JVM, and you can't really write code that relies on that in many environments. Sometimes you'll see an implementation that might have a problem if one thread tries to use it while another is configuring it, however, but even that is rare.

    Imagine a servlet that uses SOAPConnectionFactory. It's impossible to know that there isn't some other Web application in the same JVM that isn't also using it at the same time, so it has to be thread-safe.

    So, really, MessageFactory.newInstance() and SOAPConnectionFactory.newInstance() would just be wrong if they weren't thread-safe. I would use them in multiple threads without worry, and just check the source if you're really concerned. But really they're fine.

    On the other hand, objects (even other factories) created by static factory methods are often not thread safe, and you shouldn't assume that they are without documentation that says so. Even checking the source isn't sufficient, because if the interfaces aren't documented to be thread safe, then someone can add unsafe state to an implementation later.