Java Threads incrementing and decrementing an int at the same time

Question

since this is my first question ever on stackoverflow, I will try to explain it as good as possible.

I am sorry if this is a duplicate question, but I spent a lot of time searching and couldn't find an answer to this. Since i started learning Threads not so long ago I came upon an obstacle now:D I want to code a NOT thread safe method, using two threads to increment and decrement an integer at the same time.

so my code so far is this .. sadly is not working and i don't know why

public class  ThreadFunctions {
    private  int counter = 0;
    private boolean command =  false;

    public synchronized void changeNumber(boolean command){
        this.command = command;
        synchronized (this) {
            if(command){
                counter++;
            }else{
                counter--;
            }
        }
    }

    public synchronized int getCounter(){
        return this.counter;
    }
}

And that's the class I use to test it.

 public class Demo{
    public static void main(String[] args){

    final ThreadFunctions o =  new ThreadFunctions();

    new Thread(new Runnable() {
        @Override
        public void run() {
            while(o.getCounter() < 100){
                o.changeNumber(true);
                System.out.println("Thread: " + Thread.currentThread().getId() + " counter: "+o.getCounter());
            }
        }
    }).start();

    new Thread(new Runnable() {
        @Override
        public void run() {
            while(o.getCounter() > -100){
                o.changeNumber(false);
                System.out.println("Thread: " + Thread.currentThread().getId() + " counter: "+ o.getCounter());
            }
        }
    }).start();
    }
}

and the results are something like this ...

Thread: 10 counter: 5
Thread: 10 counter: 6
Thread: 10 counter: 7
Thread: 10 counter: 8
Thread: 10 counter: 9
Thread: 11 counter: 8
Thread: 10 counter: 9
Thread: 10 counter: 10
Thread: 10 counter: 11
Thread: 10 counter: 11
Thread: 10 counter: 12
Thread: 10 counter: 13
Thread: 11 counter: 13

etc..

So as u can see the threads are still not synchronised and i don't understand why:(

Multi-threaded code is hard for good programmers to write. You should not be using bare JDK 1.0 constructs like synchronized and Thread. I'd recommend newer classes from java.util.collections and java.util.concurrent. — duffymo, May 02 '16 at 14:41
BTW the block synchronized (this) is useless as you use the synchronized keyword at the method definition level — Nicolas Filotto, May 02 '16 at 14:55
You said, "as u can see..."; but we _can't_ see because you haven't told us how the output that you got is different from the output that you expected to get. — Solomon Slow, May 02 '16 at 15:09
@Palmen Penchev, this question may be useful to you: http://stackoverflow.com/questions/4818699/practical-uses-for-atomicinteger — Ravindra babu, May 02 '16 at 16:27

Nicolas Filotto · Answer 1 · 2016-05-02T15:31:27.553

To ensure the atomicity of an increment/decrement operation you can should us an AtomicInteger instead.

In your case to ensure the atomicity, instead of incrementing/decrementing and then getting the value which are not done atomically as they are not done within the same synchronized block, you should only use one method to do both like this:

public synchronized int changeNumber(boolean command){
    this.command = command;
    if (command){
        counter++;
    } else {
        counter--;
    }
    return counter;
}

Then your code executed by the threads will be:

while(o.getCounter() < 100) {
    System.out.println(
        "Thread: " + Thread.currentThread().getId() + " counter: " + o.changeNumber(true)
    );
}

and

while(o.getCounter() > -100) {
    System.out.println(
        "Thread: " + Thread.currentThread().getId() + " counter: " + o.changeNumber(false)
    );
}

score 0 · Answer 2 · answered May 02 '16 at 14:52

0

To make it it non thread safe, remove the synchronized keyword.

 public void changeNumber(boolean command){
            if(command){
                counter++;
            }else{
                counter--;
        }
    }

answered May 02 '16 at 14:52

Murat Karagöz

35,401
16
78
107

score 0 · Answer 3 · 2016-05-02T15:46:39.820

Each thread repeats increment/decrement and print. But another thread can run between increment/decrement and print like this.

OUTPUT                   THREAD 10   THREAD 11  COUNTER
----------------------   ---------   ---------  -------
Thread: 10 counter: 9    print                      9
                         increment                 10
Thread: 10 counter: 10   print                     10
                         increment                 11
Thread: 10 counter: 11   print                     11
                                     decrement     10
                         increment                 11
Thread: 10 counter: 11   print                     11
                         increment                 12
Thread: 10 counter: 12   print                     12
                         increment                 13
Thread: 10 counter: 13   print                     13
Thread: 11 counter: 13               print         13

If you want to avoid this, you should synchronize increment/decrement and print like this.

    public synchronized void changeNumber(boolean command) {
        this.command = command;
        synchronized (this) {
            if (command) {
                counter++;
            } else {
                counter--;
            }
            System.out.println("Thread: " + Thread.currentThread().getId() + " counter: " + counter);
        }
    }

Java Threads incrementing and decrementing an int at the same time

3 Answers3