Weird multithreading behaviour (Java)

Question

Just messing around with multithreading to learn more about it with regards to the synchronized keyword and other aspects I wrote a simple class and a main class to call methods of that class in different threads. I saw what I was expecting a few times (values changing by 2 instead of just 1) but then came across a larger jump which I can't understand - does anyone know how it happened?

Here's my code:

public class SeatCounter {
    private int count = 0;

    public  int getSeatCount() {
        return count;
    }

    public  void bookSeat() {
        count++;
    }

    public  void unBookSeat() {
        count--;
    }
}

and

public class Main {
    public static void main(String args[]) {
        final SeatCounter c = new SeatCounter();

        Thread t1 = new Thread() {
            public void run() {
                while(true) {
                    c.bookSeat();
                    System.out.println(c.getSeatCount());
                }
            }
        };

        Thread t2 = new Thread() {
            public void run() {
                while(true) {
                    c.unBookSeat();
                    System.out.println(c.getSeatCount());
                }
            }
        };

        t1.start();
        t2.start();
    }
}

and I got this at some point in my console:

-16066
-16067
-16068
-16069
-16031
-16069
-16068
-16067
-16066

EDIT: Thanks for the swift replies, I know about using synchronized, I was just playing around to see what went wrong, the thing I don't understand is how it changed by a jump of so much (-16069 to -16031) in one jump, since a printout statement should happen with every change of the stored value and with 2 threads running without synchronization I would've assumed that would mean at most a jump of 2 in the value.

Answer 1

There are two things that will cause "strange" counts.

• It is entirely possible for your count++, count-- calls to fail to work as expected: since count++ requires the count value to be read, and then incremented (internally: two operations), the other thread can mess around with your values after you have read them but before you have changed-and-stored them, leading to a modify-after-read data race. You can solve this with synchronization or use of AtomicInteger.
• You have no guarantees regarding when the code will execute. In particular, thread1 may finish before thread2 starts, or vice-versa, or both may take weird turns. The operating system is free to allocate processor time to threads as it sees fit (and in multi-core machines, they can actually execute in parallel). As long as there is only 1 thread generating output, you generally do not notice such things; but there is a constant churn of threads starting, executing, being pre-empted, running again, and so on while a computer is working; all managed by the operating systems' scheduler.

Answer 2

Since you're using multiple threads without synchronization everything is possible.

You have to understand how that is implemented by the VM. What might happen when multiple threads (and on multiple CPUs) are working on the same variable, is that each of them might create a local copy of the variable and therefore the behavior might be very unpredictable.

As for System.out.println() - if you want to print every single value of the SeatCounter I would suggest you to change the logic a little bit:

public class SeatCounter {
    private Object lock = new Object();
    private int count = 0;

    public SeatCounter() {
        System.out.printf("%d\t%d <- initial value\n", System.nanoTime(), count);
    }

    public int bookSeat(Thread who) {
        synchronized (lock) {
            System.out.printf("%d\t%d <- %s\n", System.nanoTime(), count, who.getName());
            count++;
            return count;
        }
    }

    public int unBookSeat(Thread who) {
        synchronized (lock) {
            System.out.printf("%d\t%d <- %s\n", System.nanoTime(), count, who.getName());
            count--;
            return count;
        }
    }

    public static void main(String args[]) {
        final SeatCounter c = new SeatCounter();

        Thread t1 = new Thread() {
            public void run() {
                while(true) {
                    c.bookSeat(this);
                }
            }
        };

        Thread t2 = new Thread() {
            public void run() {
                while(true) {
                    c.unBookSeat(this);
                }
            }
        };

        t1.start();
        t2.start();
    }
}

Here some interesting reads: On use new Atomic variables - see JavaDoc for more info: https://docs.oracle.com/javase/tutorial/essential/concurrency/atomicvars.html

Good thread here with explanations (credits to @Tomasz Nurkiewicz): What is the difference between atomic / volatile / synchronized?

Answer 3

You have no concurrency control over count. If 2 threads access the same variable at the same time it causes undefined behavior. Also When you try to access the console by using System.out.println() You have to lock around it or it outputs mangled and interleaved print statements. A lock is an object that makes sure a shared part of memory isn't changed by one thread while another is trying to read it. This is what I think you want.

import java.util.concurrent.locks.Lock;
public class Main {
public static void main(String args[]) {

    final SeatCounter c = new SeatCounter();
    Lock mylock = new Lock();
    Thread t1 = new Thread() {
        public void run() {
            while(true) {
                mylock.lock();
                c.bookSeat();
                System.out.println(c.getSeatCount());
                mylock.unlock();
            }
        }
    };

    Thread t2 = new Thread() {
        public void run() {
            while(true) {
                mylock.lock();
                c.unBookSeat();
                System.out.println(c.getSeatCount());
                mylock.unlock();
            }
        }
    };

    t1.start();
    t2.start();
}
}