3

I'm reading "Java Concurrency in Practice" and trying to write a piece of code that will show that the class presented as an example in chapter 3.5.1 can indeed introduce problems.

public class Holder {
    public int n;

    public Holder(int n) {
        this.n = n;
    }

    public void assertSanity() {
        if (n != n) {
            throw new AssertionError("sanity check failed!!");
        }
    }
}

It's said there that if used in the following way(I believe it's about the fact that the field is public, a concurrency problem may happen.

public Holder holder;
public void initialize() {
    holder = new Holder(42);
}

So I've come up with this code to see if anything bad happens.

public class SanityCheck {

    public Holder holder;

    public static void main(String[] args) {

        SanityCheck sanityCheck = new SanityCheck();
        sanityCheck.runTest();

    }

    public void runTest() {
        for (int i = 0; i < 100; i++) {
            new Thread() {
                @Override
                public void run() {
                    while (true) {
                        if (holder != null) {
                            holder.assertSanity();
                        }

                        try {
                            Thread.sleep(1);
                        } catch (InterruptedException e) {
                        }
                    }
                }
            }.start();
        }

        try {
            Thread.sleep(1000);
        } catch (InterruptedException e) {
        }

        initialize();

    }

    public void initialize() {
        holder = new Holder(42);
    }
}

But nothing bad happens, no AssertionError has been thrown.

Could you please help me figure out why this code doesn't brake anything?

Thank you in advance for your time.

Jayan
  • 18,003
  • 15
  • 89
  • 143
Michal Fotyga
  • 91
  • 1
  • 4

3 Answers3

3

The fact that the code is not thread safe and could create concurrency issues does not mean that it will do so.

The Java Memory Model (JMM) says how a program must behave when it is properly synchronized. But it does not say much about how a program could behave when it is not.

For example, a JVM that would enforce sequential consistency would be compliant with the JMM and no concurrency issue would ever happen.

W.r.t. your specific example, it is very unlikely to break on an x86/hostpot combination.

assylias
  • 321,522
  • 82
  • 660
  • 783
2

It's said there that if used in the fallowing way(I believe it's about the fact that the field is public, a concurrency problem may happen.

Problem may happen, but there is no guarantee there will be any problems.

If you are using the Oracle JVM, AFAIK it treats interoperated code accesses as volatile. It is only once you compile the constructor and the checker than you might see a problem. Even then, I suspect you will have difficultly.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
0

Shouldn't you be changing the value of n to cause something to break? In it's current form, I don't see how the AssertionError will ever be thrown, regardless of concurrency issues.

I expected something like this:

if (holder != null) {
    holder.n = holder.n - 1;
    holder.assertSanity();
}
qxn
  • 17,162
  • 3
  • 49
  • 72
  • The code could theoretically fail even without modification of `Holder.n`, since the sanity-checking thread could see a partially-initialised version of `holder` where `n` is zero, before complete initialisation is visible. And with the given code, there is no guarantee that the auxiliary thread will /ever/ see the changes made in the main thread, never mind in a consistent state! – rxg Jul 18 '13 at 20:47