Unsafe Java

Pop quiz. Why is the following Java 8 code unsafe? UPDATE: this code is fine, see comments. Still good to think about, though.

Entity e = new Entity();
e.setName("my new entity");
persistanceLayer.put(e);

To provide some context, Entity is a POJO representing something we want to store in a database. And persistanceLayer is an instance of a Data Access Object responsible for storing the object in the database. Don’t see it yet?

Here’s the function signature of that put method

@Async
<E extends AbstractEntity> CompletableFuture<E> put(E newEntity);

Ah, yes, Spring, which because of the @Async annotation will proxy this method and cause it to execute in a different thread. The object getting passed in is clearly mutable, and by my reading of the Java Memory Model, there’s no guarantee that the other thread’s view of the update that happened in the setName method will be visible. In other words, there’s no guarantee the two threads idea of “before-ness” will be the same; who knows what might have been reordered or cached?

I’m having trouble coming to terms with what seems like a gaping hole. Do you agree with this analysis? If so, how do you deal with it? Immutable object classes seem like an obvious choice, but will introduce lots of overhead in other parts of the code that need to manipulate the entities. A builder pattern gets awkward and verbose quickly, as it hoists all that complexity on to the caller.

How do you handle situations like this? I’d love to hear it.

If you produce libraries, please include documentation on what is and isn’t guaranteed w.r.t. async calls. -m

2 Responses to “Unsafe Java”

  1. Brian Goetz

    While I haven’t read the Spring proxying code, think about how the request will be proxied to another thread. The easy and sensible way is to put the entity on a queue for processing. If the queue is properly written (such as a LinkedBlockingQueue), then putting something on a queue will happen-before removing it from the queue. And since the setName happens-before the enqueue in the initiating thread, and the dequeue happens-before the use, then by transitivity the setName happens-before the use, and all is good.

    The problem would come if the client (or any other thread) mutated the Entity while the async operation was in progress, but your example doesn’t show that.

    See Java Concurrency in Practice, section 5.3.2 (serial thread confinement), among others.

  2. mdubinko

    This recent StackOverflow thread seems to agree: http://stackoverflow.com/questions/37083262/is-spring-async-pass-object-thread-safe-to-previous-update

    So it would seem there’s no immutability requirement for objects passed into (or returned from, I guess) @Async methods.

    Thanks all!

MicahLogic is Stephen Fry proof thanks to caching by WP Super Cache