Search code examples
javasemaphore

why does it cause deadlock when I use Integer to computation?


I am try to solve the problem called across Single-plank bridge. Here is the description of the problem:

There is a single-plank bridge. Only one person can cross the bridge without any stop. The pedestrian can cross the bridge from west to east or from east to west. If there is more than 0 pedestrian crossing the bridge from west to east, no pedestrian can try to cross the bridge from east to west, but more than 1 pedestrian can cross the bridge from west to east, and vice versa. The pedestrian can cross the bridge immediately if there is no pedestrian on the bridge. Use Semaphore to solve this problem.

If I use Integer, deadlock will occur sometime. When deadlock occurs, the value of wait semaphore,leftCnt and westCnt are 0, but it will release wait semaphore when leftCnt or westCnt equals zero. But if I use MyInt, everything goes will.

My solution as follow:

import java.util.concurrent.Semaphore;
//person from east to west
public class Task1 implements Runnable{
    Semaphore east, west, wait;
//    MyInt westCnt, eastCnt;
//    public Task1(Semaphore east, Semaphore west, Semaphore wait, MyInt westCnt, MyInt eastCnt) {
//        this.east = east;
//        this.west = west;
//        this.wait = wait;
//        this.westCnt = westCnt;
//        this.eastCnt = eastCnt;
//    }
    Integer westCnt, eastCnt;
    public Task1(Semaphore east, Semaphore west, Semaphore wait, Integer westCnt, Integer eastCnt) {
        this.east = east;
        this.west = west;
        this.wait = wait;
        this.westCnt = westCnt;
        this.eastCnt = eastCnt;
    }

    @Override
    public void run() {
        try {
            east.acquire();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
//        eastCnt.inc();
//        if(eastCnt.getInteger() == 1) {
//            try {
//                wait.acquire();
//            } catch (InterruptedException e) {
//                e.printStackTrace();
//            }
//        }

        eastCnt++;
        if(eastCnt.equals(1)) {
            try {
                wait.acquire();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
        System.out.println("E -> W, on bridge");
        east.release();

        try {
            east.acquire();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.out.println("E -> W, finish");
//        eastCnt.dec();
//        if(eastCnt.getInteger() == 0)
//            wait.release();
        eastCnt--;
        if(eastCnt.equals(0))
            wait.release();
        east.release();
    }
}
import java.util.concurrent.Semaphore;
//person from west to east
public class Task2 implements Runnable{
    Semaphore east, west, wait;

//    MyInt westCnt, eastCnt;
//    public Task2(Semaphore east, Semaphore west, Semaphore wait, MyInt westCnt, MyInt eastCnt) {
//        this.east = east;
//        this.west = west;
//        this.wait = wait;
//        this.westCnt = westCnt;
//        this.eastCnt = eastCnt;
//    }
    Integer westCnt, eastCnt;
    public Task2(Semaphore east, Semaphore west, Semaphore wait, Integer westCnt, Integer eastCnt) {
        this.east = east;
        this.west = west;
        this.wait = wait;
        this.westCnt = westCnt;
        this.eastCnt = eastCnt;
    }
    @Override
    public void run() {
        try {
            west.acquire();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
//        westCnt.inc();
//        if(westCnt.getInteger() == 1) {
//            try {
//                wait.acquire();
//            } catch (InterruptedException e) {
//                e.printStackTrace();
//            }
//        }
        westCnt++;
        if(westCnt.equals(1)) {
            try {
                wait.acquire();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
        System.out.println("W -> E, on bridge");
        west.release();

        try {
            west.acquire();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        System.out.println("W -> E, finish");
//        westCnt.dec();
//        if (westCnt.getInteger() == 0)
//            wait.release();
        westCnt--;
        if(westCnt.equals(0))
            wait.release();
        west.release();
    }
}
//test
public class TestApplication {

    public static void main(String args[]) {
        Semaphore east = new Semaphore(1);
        Semaphore west = new Semaphore(1);
        Semaphore wait = new Semaphore(1);
//        MyInt eastInt = new MyInt();
//        MyInt westInt = new MyInt();
        Integer eastInt = 0;
        Integer westInt = 0;
        List<Thread> list1 = new ArrayList<>();
        List<Thread> list2 = new ArrayList<>();

        for (int i = 0; i < 100; i++) {
            list1.add(new Thread(new Task1(east,west,wait, eastInt, westInt)));
            list2.add(new Thread(new Task2(east,west,wait,eastInt, westInt)));
        }
        for (int i = 0; i < list1.size(); i++) {
            list1.get(i).start();
            list2.get(i).start();
        }
    }
}
public class MyInt {
    private int integer = 0;
    public void setInteger(int integer) {
        this.integer = integer;
    }

    public int getInteger() {
        return integer;
    }
    public void inc(){
        integer++;
    }

    public void dec(){
        integer--;
    }
}

Solution

  • Integer is immutable so every time the value changes you have a different instance (by definition). Use AtomicInteger instead of reinventing it.