views:

164

answers:

3

Assume you have a Collection(ConcurrentLinkedQueue) of Runnables with mutable state. Thread A iterates over the Collection and hands the Runnables to an ExecutorService. The run() method changes the Runnables state. The Runnable has no internal synchronization.

The above is a repetitive action and the worker threads need to see the changes made by previous iterations. So a Runnable gets processed by one worker thread after another, but is never accessed by more than one thread at a time -> a case of serial thread confinement(i hope ;)).

The question: Will it work just with the internal synchronization of the ConcurrentLinkedQueue/ExecutorSerivce?

To be more precise: If Thread A hands Runnable R to worker thread B and B changes the state of R, and then A hands R to worker thread C..does C see the modifications done by B?

EDIT: Also because of the quite different answers this question keeps me busy...from JCIP, 16.2.2 Safe publication, p. 346:

[...] If thread A places X on a BlockingQueue(and no thread subsequently modifies it) and thread B retrieves it from the queue, B is guaranteed to see X as A left it. This is because the BlockingQueue implementations have sufficient internal synchronization to ensure that the put happens-before the take.[...]

So because of how ExecutorService is implemented, the only guarantee given is that the worker threads always see the Runnables as the submitting thread left them.

Back to my scenario. First A hands R to B via ExecutorService, everything is fine, B sees an up-to-date R("BlockingQueue guarantee"). Now B modifies R and then A hands it to C. So what is needed is R as B left it. But what we get is R as A left it.

The changes done in B are not guaranteed to be visible in A, even when B finishes executing R before A hands it to the ExecutorService. There is no synchronization between A and B after R was run. Perhaps B loaded a variable to some sort of local cache and updated it there.

So if A does not see the current state of R after B executed it, how can C? What is missing is the safe publication from the worker threads back to A.

If I should be wrong, that would mean that modifications done by B are visible to A altough B does not do synchronization besides the take. Where hides that guarantee?

+1  A: 

So if I understand you correctly, the queue itself is not changed (no elements are added or removed to/from it), only its elements are modified, at most by one thread ata time. And these elements (Runnables) are not thread-safe.

I think you may still run into a problem with visibility of changes between different threads. If Thread A induced a change in a Runnable R, there is no guarantee that the next thread, B (or any other thread ever, for that matter!) will see the change(s) made by Thread A unless R itself is thread-safe.

More precisely, if a field R.f is modified, the modified value of f is guaranteed to be visible to other threads only if f is declared as volatile, or it is only accessed via synchronized blocks (or if it is declared final, but then obviously you can't change its value - only the state of the referenced object if f is a reference. In which case the question becomes whether or not the referenced object itself is thread-safe).

Update: you ask in your comment:

how can I achieve what I want besides making the Runnable thread-safe?

What you want is practically to make your Runnable thread-safe regarding visibility. So your question is almost a contradiction in terms. Quoting from Java Concurrency in Practice, section 3.1.3. Locking and Visibility:

Intrinsic locking can be used to guarantee that one thread sees the effects of another in a predictable manner [...]. When thread A executes a synchronized block, and subsequently thread B enters a synchronized block guarded by the same lock, the values of variables that were visible to A prior to releasing the lock are guaranteed to be visible to B upon acquiring the lock. In other words, everything A did in or prior to a synchronized block is visible to B when it executes a synchronized block guarded by the same lock. Without synchronization, there is no such guarantee.

And from section 3.1.4. Volatile Variables:

The visibility effects of volatile variables extend beyond the value of the volatile variable itself. When thread A writes to a volatile variable and subsequently thread B reads that same variable, the values of all variables that were visible to A prior to writing to the volatile variable become visible to B after reading the volatile variable. So from a memory visibility perspective, writing a volatile variable is like exiting a synchronized block and reading a volatile variable is like entering a synchronized block. However, we do not recommend relying too heavily on volatile variables for visibility; code that relies on volatile variables for visibility of arbitrary state is more fragile and harder to understand than code that uses locking.

Use volatile variables only when they simplify implementing and verifying your synchronization policy; avoid using volatile variables when verifying correctness would require subtle reasoning about visibility. Good uses of volatile variables include ensuring the visibility of their own state, that of the object they refer to, or indicating that an important lifecycle event (such as initialization or shutdown) has occurred.

The bottom line of all this is: if you want your class to be thread safe, it is best to make it thread safe :-) Note that even if you can't modify the code of the original Runnable class, you can still create a thread-safe wrapper around it and publish it via the wrapper, effectively making its use thread-safe.

However, if (for some reason unknown to me) you don't want or can't make it fully thread-safe, you can (at your own risk) try playing around with the rules explained above: if you can organize your code such that the order of updates to the fields of your Runnable R is always the same accross all threads, you could try declaring the last modified field volatile (or its accessors synchronized); this would then in theory guarantee that all the other modifications to other fields become visible to other threads together with the update of that last field. To me, such trickery clearly falls into the category which - according to the advice quoted in bold above - should be avoided.

Péter Török
+2  A: 

What you need to quesiton yourself is whether or not there is a synchronization point. That can occur on volatile writes, thread starting, intrinsic locking and j.u.c.Lock locking. On all of those occasions any field updated will be seen by all other threads.

Considering that none of the fields are volatile or properly synchronized on, also considering that the executor service re uses threads (so no start() is invoked), I would assume that the fields updated may not be seen by all threads under all occasions.

But what is properly synchronized is the executor service. That service uses a BlockingQueue which does j.u.c.Lock locking. So when one runnable is executed, there is a synchronization point when the runnable is added and removed from that worker queue.

John V.
So there is a synchronization point when submitting the Runnable to the ExecutorService. Doesn't that mean that my described scenario works? A submits R -> synchronization point -> R gets executed -> A submits R -> synchronization point -> R gets executed -> ...? Or did I get it wrong?
denis
There is a synchronization point when submitting each runnable, you are correct. It was important to know why there is though. Lets say for example, the ExecutorService switches from BlockingQueue to a TransferQueue in Java 7, then there is no garuntees that the fields will be visible because TransferQueue is non blocking.
John V.
+1  A: 

Assuming in your implementation, A needs to know that B is done with R, before handing R to C.

From the javadoc of ExecutorService, that implementation is properly synchronized. Changes made in B is visible in C.

irreputable