Блог

Это код, который я запускаю:

 public class MyRunnableClass implements Runnable {

    static int x = 30;
    int y = 0;

    @Override
    public void run() {
        for(int i=0;i<30;i  ){
            getFromStash();
        }
    }

    public synchronized void getFromStash(){
        x--;
        y  ;
    }
}
  

и мой тестовый класс:

 public class MyRunnableClassTest {

    public static void main(String[] args){

        MyRunnableClass aa = new MyRunnableClass();
        MyRunnableClass bb = new MyRunnableClass();

        Thread a = new Thread(aa);
        Thread b = new Thread(bb);

        a.start();
        b.start();

        System.out.println(aa.y);
        System.out.println(bb.y);

    }
}
  

Иногда я вижу вывод:

 30
30
  

и иногда я вижу:

 30
0
  

Почему? Метод, который у меня есть, синхронизирован?
На самом деле я ожидаю увидеть что-то вроде 15-15, но это определенно не то, что я получаю.

Вам нужно дождаться завершения потоков.

 a.start();
b.start();

a.join();
b.join();

System.out.println(aa.y);
System.out.println(bb.y);
  

В этот момент вы должны увидеть предсказуемые результаты.

Добавлено

Теперь у вас была возможность поиграть — вот моя попытка того, что вы, похоже, пытаетесь сделать.

 public class MyRunnableClass implements Runnable {

    static AtomicInteger stash = new AtomicInteger(1000);
    int y = 0;

    @Override
    public void run() {
        try {
            while (getFromStash()) {
                // Sleep a little 'cause I'm on a single-core machine.
                Thread.sleep(0);
                // Count how much of the stash I got.
                y  = 1;
            }
        } catch (InterruptedException ex) {
            System.out.println("Interrupted!");
        }
    }

    public boolean getFromStash() {
        // It must be > 0
        int was = stash.get();
        while (was > 0) {
            // Step down one.
            if (stash.compareAndSet(was, was - 1)) {
                // We stepped it down.
                return true;
            }
            // Get again - we crossed with another thred.
            was = stash.get();
        }
        // Must be 0.
        return false;
    }
}
  

Удалите bb и используйте только aa объект для создания двух потоков.

Он синхронизирован на this , и вы используете два разных объекта (т.е. this значения) — aa и bb . Таким образом, практически вы разрушаете всю идею синхронизации, используя два разных объекта.

     Thread a = new Thread(aa);
    Thread b = new Thread(aa);

    a.start();
    b.start();
  

В качестве альтернативы вы можете сделать что-то вроде этого.

 public class MyRunnableClass implements Runnable {
    private static final Object lock = new Object();

    static int x = 30;
    int y = 0;

    @Override
    public void run() {
        for(int i=0;i<30;i  ){
            getFromStash();
        }
    }

    public void getFromStash(){
        synchronized(lock){
            x--;
            y  ;
        }
    }
}
  

Вот чего, я думаю, вы хотите достичь.

 class Stash {

    private int x = 30;
    private int y = 0;

    public int getX() {
        return x;
    }

    public void setX(int x) {
        this.x = x;
    }

    public int getY() {
        return y;
    }

    public void setY(int y) {
        this.y = y;
    }

    public synchronized void getFromStash(){
        System.out.println("Method getFromStash called by "   Thread.currentThread().getName()   ".");
        x--;
        y  ;
    }
}

public class MyRunnableClass implements Runnable {

    private Stash st = null;
    private volatile boolean done = false;

    public MyRunnableClass(Stash st){
        this.st = st;
    }

    @Override
    public void run() {
        for(int i=0;i<30;i  ){
            this.st.getFromStash();
            try {
                double m = Math.random();
                Thread.sleep((long)((m   1) * 100.0));
            }catch(InterruptedException ex){
                ex.printStackTrace();
            }
        }
        System.out.println("Thread ---> "   Thread.currentThread().getName()   " finished!");
        this.done = true;
    }

    public static void main(String[] args) throws Exception {
        Stash st = new Stash();

        MyRunnableClass aa = new MyRunnableClass(st);
        MyRunnableClass bb = new MyRunnableClass(st);

        Thread a = new Thread(aa);
        Thread b = new Thread(bb);
        a.setName("Thread A");
        b.setName("Thread B");
        a.start();
        b.start();

        while (true){
            System.out.println(st.getX()   " "   st.getY());
            Thread.sleep(10);
            if (aa.done amp;amp; bb.done) break;
        }
        System.out.println("Main thread finished too!");
    }
}
  

Поскольку вы печатаете значения сразу после запуска потоков, вы не собираетесь «перехватывать» потоки в середине циклов for. Планировщик потоков возвращает управление основному потоку иногда после завершения потоков, а иногда до их запуска, но никогда во время run() . Вам нужно дождаться завершения потоков.

Как вы уже выяснили, ваша первая попытка сработала не так, как вы хотели, потому что 1) вы не ждали завершения потоков, поэтому иногда вы считываете значения до того, как они выполнили свою работу, и 2) вы не хотите, чтобы каждый поток извлекал данные из хранилища 30 раз, а скорее, чтобы общая сумма извлечений составляла 30 (разделенная между потоками, как бы это ни происходило).

Ваш переход к остановке каждого потока при x> 0 вместо после N нажатий является правильным подходом, но проверка того, является ли x> 0 (и, следовательно, продолжать ли), также должна быть синхронизирована. В противном случае вы могли бы протестировать значение и обнаружить, что x = = 1, решить выполнить извлечение, а затем, прежде чем вы действительно это сделаете, другой поток возьмет последнее. Затем вы выполняете свой pull, оставляя x равным -1, а сумму двух y равной 31.

Чтобы решить эту проблему, вам либо нужно установить проверку на x > 0 в синхронизированном методе getFromStash() (таким образом, вы фактически не изменяете x и y, если это не безопасно), либо вам нужно выставить блокировку за пределами объекта Stash из ответа peter.petrov , чтобы оба потока могли явно синхронизировать этот объект при проверке x > 0, а затем вызвать getFromStash (), если применимо.

Кроме того, обычно намного сложнее определить синхронизацию потоков, когда вы используете статические переменные; как правило, происходят взаимодействия, которых вы не ожидаете. Гораздо лучше создать отдельный объект (например, класс Stash peter.petrov), который поможет вам представлять пул, и передать ссылку на него каждому из ваших классов потоков. Таким образом, весь доступ осуществляется через нестатические ссылки, и вам будет легче убедиться, что вы получили правильный код.