Многопоточность не завершается?

Question

[Павел Дуденков]

Данный код зависает, есть поменять значение totalCountSpeeches на 2000 - завершается.
У меня есть несколько предположений, но они лишь догадки, поскольку не хватает знаний как работает JVM с несколькими потоками...

public class Solution {
    public static int totalCountSpeeches = 200;
    public static int soundsInOneSpeech = 1000000;

    public static void main(String[] args) throws InterruptedException {
        Politic ivanov = new Politic("Иванов");
        Politic petrov = new Politic("Петров");
        Politic sidorov = new Politic("Сидоров");

        while (ivanov.getCountSpeaches() + petrov.getCountSpeaches() + sidorov.getCountSpeaches() < totalCountSpeeches) {
        }

        System.out.println(ivanov);
        System.out.println(petrov);
        System.out.println(sidorov);
    }

    public static class Politic extends Thread {
        private int countSounds;

        public Politic(String name) {
            super(name);
            start();
        }

        public void run() {
            while (countSounds < totalCountSpeeches * soundsInOneSpeech) {
                countSounds++;
            }
        }

        public int getCountSpeaches() {
            return countSounds / soundsInOneSpeech;
        }

        @Override
        public String toString() {
            return String.format("%s сказал речь %d раз", getName(), getCountSpeaches());
        }
    }
}

Comments

[razer89]

Вероятнее всего, у вас ошибка в логике. Где инициализируется переменная countSound?

[Павел Дуденков]

razer89: явно нигде, поскольку это int, то дефолт значение у неё 0. При вызове конструктора она неявно инициализируется значением 0 после того, как отработает super(name).

З.Ы. я могу ошибаться, опираюсь на свои знания

[Павел Дуденков]

razer89: попробовал её явно инициализировать во всех местах - при обьявлении, явно в конструкторе перед вызовом старт и в начале метода run.
Результат - тот же

Answer 1

[pbahushevich]

Runnable картины не изменит.
Во-первых комментарии по коду

while (ivanov.getCountSpeaches() + petrov.getCountSpeaches() + sidorov.getCountSpeaches() < totalCountSpeeches) {
        }

тут я так понимаю ты ждешь когда все вместе наберут 200 голосов и после этого цикл прекратится и у тебя будут данные о том кто сколько голосов набрал.
Проблема в том что конец цикла происходит в потоке main, а остальные потоки никак об этом не знают и продолжат работать. И между окончанием цикла и выводом результатов на экран 3 потока успеют много чего сделать и результаты уже будут сильно отличаться. Ну или не сильно ))) т.е. сумма ровно в 200 не сойдется.
Проблема еще в том что у тебя зависает еще до выхода из цикла и проблема эта решается в строке
private int countSounds;
добавлением волшебного слова volatile.
почему - см. Java Memory Model либо если не поймешь пиши в комменты ))

Comments

[Павел Дуденков]

Я догадывался, что может быть дело в том, что один поток может не видит изменений одной и той же переменной в другом потоке, но успокоился , когда подумал, что это private переменная, которая используется только в рамках одного потока и только он по идее в неё смотрит, хотя всё же нет, она уходит за пределы потока методом getCountSpeaches. Прочитаю про модель памяти, спасибо :) Отпишусь после

[Павел Дуденков]

pbahushevich кстати, насчет замечаний про некоторое несоответствие кода и ожиданий - это я осознаю. Этот цикл скорее сделан для того, чтобы сделать вывод в консоль с количеством голосов в тот момент, когда они вместе соберут хотя бы 200. Но на самом - конечная задача изначально было в том, чтобы заставить один из трёх потоков набрать больше, чем остальные. Этого добиться получилось :) А потом вот вылез этот нюанс, который я ещё не осознал.

[pbahushevich]

Павел Дуденков: а понял, просто создается впечатление что кандидаты должны разделить 200 голосов между собой))

[Павел Дуденков]

pbahushevich: всё, понял в чем проблема. Именно из-за потоко-небезопасного изменения\доступа переменной:
private int countSounds;
Цикл в main подвисал и не заканчивался. После volatile - всё чётко синхронизируется и видит друг друга :) Спасибо!

[Павел Дуденков]

pbahushevich: убрал volatile, добавил вывод в цикле main и внутри класса Politic в методе getCountSpeaches:

while (ivanov.getCountSpeaches() + petrov.getCountSpeaches() + sidorov.getCountSpeaches() < totalCountSpeeches) {
            System.out.println(ivanov.getName()+" -> "+ivanov.getCountSpeaches());

public int getCountSpeaches() {
            System.out.println("in class: "+this.getName()+" -> "+this.countSounds);

И получил ясный итог:
in class: Иванов -> 1199
Иванов -> 0

[pbahushevich]

Павел Дуденков: о, "visibility problem" прям как на картинке )))))

[Павел Дуденков]

pbahushevich: и всё же, даже при использовании synchronyzed and volatile + static variable у меня не получилось добиться того, чтобы они вместе набирали какое-то чётко-заданное количество, вот этот код:

public class Solution {
    public static int totalCountSpeeches = 200000;
    public static int soundsInOneSpeech = 1;

    public static void main(String[] args) throws InterruptedException {
        Politic ivanov = new Politic("Иванов"); //ivanov.join();
        Politic petrov = new Politic("Петров");
        Politic sidorov = new Politic("Сидоров");

//        while (Politic.totalSounds < totalCountSpeeches*soundsInOneSpeech) {
//            System.out.println("in main: total sounds -> "+Politic.totalSounds);
//            System.out.println("in main: "+ivanov.getName()+" -> "+ivanov.getCountSpeaches());
//            System.out.println("in main: "+petrov.getName()+" -> "+petrov.getCountSpeaches());
//            System.out.println("in main: "+sidorov.getName()+" -> "+sidorov.getCountSpeaches());
//        }
        while (!(ivanov.finish | petrov.finish | sidorov.finish)) {}
        System.out.println(ivanov);
        System.out.println(petrov);
        System.out.println(sidorov);
    }

    public static class Politic extends Thread {
        public volatile static int totalSounds;
        private volatile int countSounds;
        public volatile boolean finish = false;

        public Politic(String name) {
            super(name);
            start();
        }

        public synchronized void run() {
            while (totalSounds < totalCountSpeeches * soundsInOneSpeech) {
                countSounds++;
                totalSounds++;
            }
            finish = true;
        }

        public int getCountSpeaches() {
            System.out.println("in class: "+Politic.totalSounds);
            System.out.println("in class: "+this.getName()+" -> "+countSounds);
            return countSounds;
        }

        @Override
        public String toString() {
            return String.format("%s сказал речь %d раз", getName(), getCountSpeaches());
        }
    }
}

[pbahushevich]

Павел Дуденков:
volatile - обещает то что операции записи и чтения будут атомарными. Т.е. если пока твой поток пишет int другой поток его не перепишет, т.к. при записи longa насколько я знаю, операция записи старших байт может происходить отдельно от операции записи младших.
i++ это операция чтения, вычисления (+1) и затем записи, т.е. тут 3 операции и volatile тут не спасает.
Для этого есть такой класс как AtomicInteger. Плюс у тебя блок сравнения totalSounds и увеличения должен находиться в одном synchronized.
synchronized метод класса - вызывается с блокировкой монитора на "this" (на текущем экземпляре). Ты же меняешь статическую переменную класса, которая не привязана к объекту, соответственно synchronized метод тут не поможет.
Если переписать вот так то работает.

public static class Politic extends Thread {
public volatile static AtomicInteger totalSounds = new AtomicInteger(0);
private volatile int countSounds;
public volatile boolean finish = false;

public Politic(String name) {
super(name);
start();
}

public void run() {
while (true) {
synchronized (totalSounds) {
if(totalSounds.get() >= totalCountSpeeches * soundsInOneSpeech) {
break;
}
countSounds++;
totalSounds.getAndIncrement();
}
}
finish = true;
}

И вместо finish - можно использовать petrov.join();
И я бы был тебе очень благодарен если бы ты разобрался, почему не работает с простым Integer
и объяснил мне.)))) потому как это я уже не понимаю....

[Павел Дуденков]

pbahushevich: вот я экспериментировал с обычным Integer, оно то работает для статика totalSounds , но вот почему-то не работает для countSounds ... Не понимаю, я даже пробовал оборачивать в два вложенных блока synchronized для этих переменных... ))

[Павел Дуденков]

Видимо Integer и int тоже меняется частями, отсюда и косяки :/

[Павел Дуденков]

pbahushevich: заметил одну интересную вещь, вот так, будет работать даже с простым Integer:

public void run() {
while (true) {
yield();
synchronized (totalSounds) {
if(totalSounds >= totalCountSpeeches * soundsInOneSpeech) {
break;
}

countSounds++;
totalSounds++;
}
try {
sleep(1);
} catch (InterruptedException e) {}
}
finish = true;
}

Видимо использование AtomicInteger заставляет JVM неявно каждый раз начинать операции с ним с начала кванта времени для одной нити. А тут я явно заставляю перед каждой итерацией цикла JVM дождаться нового кванта времени для треда.

[pbahushevich]

Урааааа - я понял )))) в Java Integer - немутабельный тип. Т.е каждый раз когда ты меняешь значение происходит примерно следующее:

Integer n =10;  // Integer n = new Integer.valueOf(10);
n = 11;             // n = new Integer.valueOf(11);
n = n+1;          // n = new Inger.valueOf(n.intValue()+1);

соответственно синхронизироваться по нему бессмысленно и происходит такая вот картина

Answer 2

[protven]

Ну а почему вы считаете что у вас "зависло" ? Добавьте отладочную печать, увидите, делает что-то ваш код или нет. Проблем то много может быть, начиная от гонки ресурсов и переполнения инта например.

while (countSounds < totalCountSpeeches * soundsInOneSpeech) {
                countSounds++;
                 System.out.println("name -> " + this.getName() + ", " + "count -> " + countSounds);
            }

Comments

[abs0lut]

Дебагером же удобнее

[Павел Дуденков]

abs0lut: дебагером всё отлично завершается. В том то и проблема, видимо оно успевает всюду перескочить и избежать гонок, когда исполняется по одной строке.