73

From page 291 of OCP Java SE 6 Programmer Practice Exams, question 25:

public class Stone implements Runnable {
    static int id = 1;

    public void run() {
        id = 1 - id;
        if (id == 0) 
            pick(); 
        else 
            release();
    }

    private static synchronized void pick() {
        System.out.print("P ");
        System.out.print("Q ");
    }

    private synchronized void release() {
        System.out.print("R ");
        System.out.print("S ");
    }

    public static void main(String[] args) {
        Stone st = new Stone();
        new Thread(st).start();
        new Thread(st).start();
    }
}

One of the answers is:

The output could be P Q P Q

I marked this answer as correct. My reasoning:

  1. We are starting two threads.
  2. First one enters run().
  3. According to JLS 15.26.1, it firstly evaluates 1 - id. Result is 0. It is stored on the thread's stack. We are just about to save that 0 to static id, but...
  4. Boom, scheduler chooses the second thread to run.
  5. So, the second thread enters run(). Static id is still 1, so he executes method pick(). P Q is printed.
  6. Scheduler chooses first thread to run. It takes 0 from its stack and saves to static id. So, the first thread also executes pick() and prints P Q.

However, in the book it's written that this answer is incorrect:

It is incorrect because the line id = 1 - id swaps the value of id between 0 and 1. There is no chance for the same method to be executed twice.

I don't agree. I think there is some chance for the scenario I presented above. Such swap is not atomic. Am I wrong?

Adam Stelmaszczyk
  • 18,835
  • 4
  • 63
  • 104
  • Did they allow for R S R S by the way? – Jon Skeet Nov 23 '14 at 13:03
  • 1
    @JonSkeet There was no such answer. They allowed for `P Q R S`, `P R S Q` and `P R Q S`, to which I agree. – Adam Stelmaszczyk Nov 23 '14 at 13:06
  • I think you took the JLS section you're referring to out of context. That section goes over simple assignments (as in a single thread). I think you need to review [JLS 17.4. Memory Model](https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.4). – hfontanez Nov 23 '14 at 13:17
  • 1
    Surely `P R S Q` and `P R Q S` aren’t possible either, since `pick` and `release` are synchronized. Am I missing something (my Java is probably a bit rusty)? – matt Nov 24 '14 at 01:16
  • 2
    In the original code example (from the book mentioned), the `release` method is _not_ static. So `P R S Q` and `P R Q S` _are_ possible solutions indeed. Nevertheless, this doesn't fix the race condition in the `run` method, hence the book is still wrong concerning this issue. – isnot2bad Nov 24 '14 at 07:39
  • @isnot2bad Thank you for correction, I've fixed the code. – Adam Stelmaszczyk Nov 24 '14 at 08:22
  • "There is no chance for the same method to be executed twice." That's proof, all by itself, that the text is bollox. Of course it's executed twice! The example starts two threads, and both threads execute it. The person who wrote "there is no chance..." does not know what he/she is talking about. – Solomon Slow Nov 24 '14 at 09:27

2 Answers2

78

Am I wrong?

Nope, you're absolutely right - as is your example timeline.

In addition to it not being atomic, it's not guaranteed that the write to id will be picked up by the other thread anyway, given that there's no synchronization and the field isn't volatile.

It's somewhat disconcerting for reference material like this to be incorrect :(

Jon Skeet
  • 1,261,211
  • 792
  • 8,724
  • 8,929
  • 1
    Thank you, but what do you mean by *it's not guaranteed that the write to `id` will be picked up by the other thread anyway*? That it can be somehow optimized and there can be no write to `id` by the second thread? I don't understand that part. – Adam Stelmaszczyk Nov 23 '14 at 13:18
  • 9
    @AdamStelmaszczyk: No, I mean that thread 1 could write a new value to `id`, but thread 2 might not see it immediately - it might see the *old* value. – Jon Skeet Nov 23 '14 at 13:31
  • 25
    Deep under the hood of your CPU lie dragons. The particular issue Jon is talking about is called "cache coherency." When you ask for the value of a variable, it would be too slow to go to main memory every time (hundreds of times slower than you want!). To deal with this, modern processors all have a CPU-specific or core-specific memory cache that they look in first. This means one thread can change the "official" value of `id` in memory, and another thread never sees it because it never looks further than its own cache. – Cort Ammon Nov 23 '14 at 21:05
  • 6
    There is a whole collection of tools to deal with cache coherency, ranging from explicit cache flush commands to atomics to synchronization. Fortunately for those who never ever ever ever ever want to deal with those dragons, synchronization with `synchronized` or mutexes generally does "what you think it should do," as long as you use them to protect your data using the standard patterns (like `synchronized` in the example above). Its just a shame they got it wrong with `id`. – Cort Ammon Nov 23 '14 at 21:07
  • 9
    Worth noting that the author even [admitted there might be something wrong with the question](http://www.coderanch.com/t/531921/java-programmer-SCJP/certification/Threads-OCP-Java-Practice-Exams#post_text_2413290) (since there is), though I guess they never bothered to put up a list of errata. – Tim Stone Nov 24 '14 at 02:43
  • 1
    There's an interesting, if poorly formatted, set of test results at the end of the thread Tim Stone linked to above (I did not look at it closely to verify that it was correctly implemented). – Jason C Nov 24 '14 at 07:15
  • @CortAmmon Actually, that's a myth. On every modern, popular CPU that supports Java and more than one core, a thread *cannot* modify a region of memory while that region of memory is cached by another core. The entire purpose of cache coherency hardware is to prohibit exactly that from happening. – David Schwartz Oct 03 '16 at 10:30
  • @DavidSchwartz That sounds very expensive. Is there a resource you can cite on that? (I know the C++ spec community spent at least half decade solving a problem in C++11 that suggests that they *can* modify memory that way) – Cort Ammon Oct 03 '16 at 14:22
  • @CortAmmon Punch "MESI protocol" into your favorite search engine. It's not expensive at all, quite the contrary, it's a tremendous cost savings because getting a value out of another core's cache is *way* cheaper than going all the way to memory even if you have to wait an instruction on rare occasions where an atomic operation has locked the cache line. (The problems you're thinking of are real, they just have nothing to do with cache coherency. That's the part that's the myth. (They have to do with things like read and write ordering.)) – David Schwartz Oct 03 '16 at 16:52
-3

In my opinion, the answer in the Practice Exams is correct. In this code, you are executing two threads which have access to the same static variable id. Static variables are stored on the heap in java, not on the stack. The execution order of runnables is unpredictable.

However, in order to change the value of id each thread :

  1. makes local copy of the value stored in id's memory address to the CPU registry;
  2. performs the operation 1 - id. Strictly speaking, two operations are performed here (-id and +1);
  3. moves the result back to memory space of id on the heap.

This means that although the id value can be changed concurrently by any of the two threads, only the initial and final values are mutable. Intermediate values will not be modified by one another.

Futhermore, analysis of the code can show that at any point in time, id can only be 0 or 1.

Proof:

  • Starting value id = 1; One thread will change it to 0 ( id = 1 - id ). And the other thread will bring it back to 1.

  • Starting value id = 0; One thread will change it to 1 ( id = 1 - id ). And the other thread will bring it back to 0.

Therefore, the value state of id is discrete either 0 or 1.

End of Proof.

There can be two possibilities for this code:

  • Possibility 1. Thread one accesses the variable id first. Then the value of id (id = 1 - id changes to 0. Thereafter, only the method pick () will be executed, printing P Q. Thread two, will evaluate id at that time id = 0; method release() will then be executed printing R S. As a result, P Q R S will be printed.

  • Possibility 2. Thread two accesses the variable id first. Then the value of id (id = 1 - id changes to 0. Thereafter, only the method pick () will be executed, printing P Q. Thread one, will evaluate id at that time id = 0; method release() will then be executed printing R S. As a result, P Q R S will be printed.

There are no other possibilities. However, it should be noted that variants of P Q R S such as P R Q S or R P Q S, etc. may be printed due to pick() being a static method and is therefore shared between the two threads. This leads to the simultaneous execution of this method which could result in printing the letters in a different order depending on your platform.

However in any case, never will either the method pick() or release () be executed twice as they are mutually exclusive. Therefore P Q P Q will not be an output.

user2399800
  • 3
  • 1
  • 2
    In your step 2 the result of `1 - d` (zero) will be stored on the thread's stack. Now, before step 3, scheduler can change the running thread. So, second thread will also have zero from `1 - d`. Please study carefully scenario I presented in a question. That's why `P Q P Q` is possible. If you are still not convinced by theoretical analysis already presented on this page, please also take a look on the last post [here](http://www.coderanch.com/t/531921/java-programmer-SCJP/certification/Threads-OCP-Java-Practice-Exams#post_text_2413290), where it's shown empirically that it's possible. – Adam Stelmaszczyk Dec 03 '14 at 22:55
  • Your "proof" omits two cases - with a starting value of 1, _both_ threads could try to change it to 0; and with a starting value of 0, _both_ threads could try to change it to 1. This answer is incorrect. – Dawood ibn Kareem Nov 06 '19 at 23:44