Concurrency, object visibility

Question

I am trying to find out if the code below contains any potential concurrency problems. In particular, the visibility problem associated with volatile variables. Volatility is defined as: The value of this variable will never be cached by the thread-locally: all reads and writes will go directly to the "main memory"

public static void main(String [] args) { Test test = new Test(); // This will always single threaded ExecutorService ex = Executors.newSingleThreadExecutor(); for (int i=0; i<10; ++i) ex.execute(test); } private static class Test implements Runnable { // non volatile variable in question private int state = 0; @Override public void run() { // will we always see updated state value? Will updating state value // guarantee future run see the value? if (this.state != -1) this.state++; } } 

For the above single-threaded artist :

Is it possible to make test.state unstable? In other words, will there be every subsequent Test.run () (which will happen sequentially, and not simultaneously, because again the executor is a single thread), do you always see the updated value of test.state? If not, will Test.run () exit so that any changes made on the local network are written to main memory? Otherwise, when changes made to the stream locally are written to main memory, if not when exiting the stream?

Answer 1

Yes, it is safe, even if the performer replaced his thread in the middle. The start / end of the stream are also synchronization points.

http://java.sun.com/docs/books/jls/third_edition/html/memory.html#17.4.4

A simple example:

 static int state; static public void main(String... args) { state = 0; // (1) Thread t = new Thread() { public void run() { state = state + 1; // (2) } }; t.start(); t.join(); System.out.println(state); // (3) } 

It is guaranteed that (1), (2), (3) are well ordered and behave as expected.

For the executor of a single thread, "Tasks are guaranteed to be executed sequentially," he must somehow determine the completion of one task before starting the next, which must correctly synchronize various run()

Answer 2

While this is only one thread, you do not need to make it volatile. If you intend to use multiple threads, you should not only use volatile, but also synchronize. Incrementing a number is not an atomic operation - it is a common misconception.

 public void run() { synchronize (this) { if (this.state != -1) this.state++; } } 

Instead of using synchronization, you can also use AtomicInteger # getAndIncrement () (if you don't need if, if before).

 private AtomicInteger state = new AtomicInteger(); public void run() { state.getAndIncrement() } 

Answer 3

Initially, I thought like this:

If the task was always performed in the same topic, there would be no problem. But the Excecutor created by newSingleThreadExecutor() can create new themes to replace those that are killed for any reason. There is no guarantee when a replacement will create a thread or which thread will create it.

If the thread performs some records, it calls start() in the new thread, those records will be visible to the new thread. But there is no guarantee that this rule applies in this case.

But it is inappropriate right: creating the right ExecutorService without sufficient barriers to ensure visibility is almost impossible. I forgot that detecting the death of another thread is syncing with relationships. The locking mechanism used for idle workflows also requires a barrier.

Answer 4

Your code in particular this bit

  if (this.state != -1) this.state++; 

An atomic test of the state value will be required, and then the state increment in a parallel context. Therefore, even if your variable was volatile and more than one thread was involved, you are having concurrency problems.

But your design is based on the assertion that there will always be only one instance of Test, and this only instance is provided to only one (same) thread. (But note that a single instance is actually a shared state between the main thread and the worker thread.)

I think you need to make these assumptions more explicit (in code, for example, use ThreadLocal and ThreadLocal.get ()). This should protect against future errors (when some other developer may neglect constructive assumptions), and protect against assumptions about the internal implementation of the Executor method that you use , which in some implementations can simply provide a single-threaded executor (i.e., sequential and not necessarily the same thread in every call to execute (runnable).

Answer 5

It is perfectly normal for the state to be non-volatile in this particular code, because there is only one thread, and only this thread accesses the field. Disabling caching of the value of this field within the only thread that you have will produce a simple result.

However, if you want to use the state value in the main thread that starts the loop, you need to make the volatile field:

  for (int i=0; i<10; ++i) { ex.execute(test); System.out.println(test.getState()); } 

However, even this may not work correctly with volatile, as there is no synchronization between threads.

Since the field is private, there is only a problem if the main thread executes a method that can access this field.

Answer 6

If your ExecutorService is single-threaded, then there is no general state, so I don’t see how problems can occur.

However, it makes no sense to pass a new instance of your Test class for each call to execute() ? i.e.

 for (int i=0; i<10; ++i) ex.execute(new Test()); 

Thus, there will be no general condition.