views:

214

answers:

5

What is the best way to test value visibility between threads?

class X {
 private volatile Object ref;

 public Object getRef() {
  return ref;
 }

 public void setRef(Object newRef) {
  this.ref = newRef;
 }
}

The class X exposes a reference to the ref object. If concurrent threads read and and write the object reference every Thread has to see the latest object that was set. The volatile modifier should do that. The implementation here is an example it could also be synchronized or a lock-based implementation.

Now I'm looking for a way to write a test that informs me when the value visibility is not as specified (older values were read).

It's okay if the test does burn some cpu cycles.

+2  A: 

If I understand correctly, then you want a test-case that passes if the variable is defined as volatile and fails if not.

However I think there is no reliable way to do this. Depending on the implementation of the jvm concurrent access may work correctly even without volatile.

So a unit test will work correctly when volatile is specified but it still might work correctly without volatile.

Wolfgang
Indeed, such a test would be unreliable
skaffman
It's in the nature of this kind of problems that they are indeterministic and depend on the characteristic of the vm implementation. To know that it works in a setting is more than to nothing about your code. If it cannot be observed you're always lost with testing. The goal is to have such a test coverage that if you removed one aspect of your code your tests will complain.
Thomas Jung
+1  A: 

Wow, that's much tougher than I initially thought. I might be completely off, but how about this?

class Wrapper {
    private X x = new X();
    private volatile Object volatileRef;
    private final Object setterLock = new Object();
    private final Object getterLock = new Object();

    public Object getRef() {
        synchronized(getterLock) {
            Object refFromX = x.getRef();
            if (refFromX != volatileRef) {
                // FAILURE CASE!
            }
            return refFromX;
        }
    }

    public void setRef(Object ref) {
        synchronized(setterLock) {
            volatileRef = ref;
            x.setRef(ref);
        }
    }
}

Could this help? Of course, you will have to create many Threads to hit this wrapper, hoping for the bad case to appear.

tim_wonil
The problem with this is that synchronized(setterLock) { volatileRef = ref; x.setRef(ref); }is not atomic for the reader.
Thomas Jung
+1  A: 

The JLS says what you are supposed to do in order to get guaranteed consistent execution in an application involving "inter-thread actions". If you don't do these things, the execution may be inconsistent. But whether it actually will be inconsistent depends on the JVM you are using, the hardware that you are using, the application, the input data, and ... whatever else might be happening on the machine when you run the application.

I cannot see what your proposed test would tell you. If the test shows inconsistent executions, it would confirm the wisdom of doing synchronization properly. But if running the test a few time shows only (apparently) consistent executions, this doesn't tell you that executions are always going to be consistent.

EDIT: Suppose that you run your tests on (say) JDK 1.4.2 (rev 12) / Linux / 32bit with the 'client' JVM and options x, y, z running on a single processor machine. And that after running the test 1000 times, you observe that it does not seem to make any difference if you leave out the volatile. What have you actually learned?

  • You have not learned that it really makes no difference? If you change the test to use more threads, etc, you may get a different answer. If you run the test a few more thousand or million or billion times, you might get a different answer.
  • You have not learned anything about what might happen on any other platforms.
  • You have not learned if it is safe to leave out the volatile keyword.

The problem is that this kind of test is the worst kind of black box testing. Unless you can examine in depth what the JIT compilers are actually doing, you have no real insight as to what is going on inside the box.

Stephen C
If you write a test for a method "String x(String y)" you do know that there is no chance to test all string values. Nonetheless you try get test a representative value of every equivalence class. This problem is more fuzzy. It depends on the vm implementation probably even on the configuration of the vm.
Thomas Jung
EDIT: Theoretically you could control the vm. It could be an implementation that is very aggressive in its interpretation of the java memory model. Its not easy and it will not be deterministic for one test run but doable and better than "this aspect is untestable".
Thomas Jung
“You have not learned ...”: Code to test won't be so easy in reality. It can be implemented in many ways (volatile, locks, synchronized, misbehaving implementations). It could be an integration test, where behavior of many units lead to visibility problems. (Which is more of a design problem but real code is at it is.) You have to find out that there is a problem in your code. Ever done cyclic dependency analysis in a code base without jdepend? Good luck! This problem is trivial in comparison and a lot of (most?) code is not right. It is not that you could learn a lot by fixing this problem.
Thomas Jung
@Thomas: what I am saying is that there is little point writing tests to test the effect of (deliberately) doing synchronization incorrectly. You KNOW that it is wrong ... so just don't do it.
Stephen C
@Thomas: and the counterpoint is that it is not really possible to test that you have done synchronization correctly.
Stephen C
A: 

How about this ?

public class XTest {

 @Test
 public void testRefIsVolatile() {
  Field field = null;
  try {
   field = X.class.getDeclaredField("ref");
  } catch (SecurityException e) {
   e.printStackTrace();
   Assert.fail(e.getMessage());
  } catch (NoSuchFieldException e) {
   e.printStackTrace();
   Assert.fail(e.getMessage());
  }
  Assert.assertNotNull("Ref field", field);
  Assert.assertTrue("Is Volatile", Modifier.isVolatile(field
    .getModifiers()));
 }

}

tartur
The test has to have no knowledge about the code. Every implementation (synchronized, locks, volatile) is okay as long as the visibility is right.
Thomas Jung
A: 

So basicaly you want this scenario: one thread writes the variable, while another reads it at the same time, and you want to ensure that the variable read has the correct value, right?

Well, I don't think you can use unit testing for that, because you can't ensure the right environment. That is done by the JVM, by how it schedules instructions. Here's what I would do. Use a debugger. Start one thread to write the data and put a breakpoint on the line that does this. Start the second thread and have it read the data, also stopping at that point. Now, step the first thread to execute the code that writes, and then read with the second one. In your example, you won't achieve anything with this, because read and write are single instructions. But usually if these operations are more complex, you can alternate the execution of the two threads and see if everything is consistent.

This will take some time, because it's not automated. But I wouldn't go and write a unit test that tries reading and writing a lot of times, hoping to catch that case where it fails, because you wouldn't achieve anything. The role of a unit test is to assure you that code you wrote is working as expected. But in this case, if the test passes, you're not assured of anyhing. Maybe it was just lucky and the conflict didn't appera on this run. And that defeats the purpose.

Andrei Vajna II