Здравствуйте.
В книжке по джаве, после описания функций wait/notify/notifyAll/sleep и др. идет такое упражнение.

Напишите программу, которая каждую секунду отображает на экране данные о времени, прошедшем от начала сессии, а другой ее поток выводит сообщение каждые 5 секунд. Предусмотрите возможность ежесекундного оповещения потока, воспроизводящего сообщение, потоком, отсчитывающим время. Не внося изменений в код потока-"хронометра" , добавьте еще один поток, который выводит на экран другое сообщение каждые 7 секунд.

Вопрос: не слишком ли извращенно я решил эту задачу? В книге методы wait/notify* применялись только по отношению к this.

class Event
{
    public boolean finish = false;
}

class notifier implements Runnable
{
    private Event event;
    private int delay = 1, elapsed = 0;
    notifier(){}
    notifier(int delay, Event event)
    {
        this.delay = delay;
        this.event = event;
    }
    public void run()
    {
        try
        {
            while(true)
            {
                synchronized(event) // *
                {
                    event.wait();
                }
                if(event.finish) return;
                ++elapsed;
                if(elapsed == delay)
                {
                    System.out.println(elapsed + " second(s) elapsed");
                    elapsed = 0;
                }
            }
        }
        catch(InterruptedException ex)
        {
        }
    }
}

public class Exercise
{
    private Event event = new Event();
    public static void main(String[] args)
    {
        System.out.println("START!");
        Exercise prog = new Exercise();
        prog.timer();
    }
    
    synchronized public void timer()
    {
        Runnable every_1 = new notifier(1, event);
        Runnable every_5 = new notifier(5, event);
        new Thread(every_1).start();
        new Thread(every_5).start();
        try
        {
            int i = 20;
            while(true)
            {
                Thread.sleep(1000);
                if(i-- < 0)
                {
                    event.finish = true;
                    synchronized(event)
                    {
                        event.notifyAll();
                    }
                    return;
                }
                synchronized(event)
                {
                    event.notifyAll();
                }
            }
        }
        catch(InterruptedException ex)
        {
        }
    }
}

И второй вопрос (если лень разбираться с упражнением, то ответьте хотя бы на этот ). Почему если убрать инструкцию synchronized(event){...}, а метод run сделать synchronized, то вылетает исключение при вызове wait? (Т.е. event оказывается не синхронизированым). Я думал что спецификатор synchronized для метода означает synchronized(this) и соответственно его поле event должно быть тоже синхронизировано?

Здравствуйте, Аноним, Вы писали:


А>И второй вопрос (если лень разбираться с упражнением, то ответьте хотя бы на этот ).

Лень


А>Почему если убрать инструкцию synchronized(event){...}, а метод run сделать synchronized, то вылетает исключение при вызове wait? (Т.е. event оказывается не синхронизированым). Я думал что спецификатор synchronized для метода означает synchronized(this) и соответственно его поле event должно быть тоже синхронизировано?

Когда ты указываешь synchronized(object), ты пытаешься взять лок только на указанный объект, т.е. если объект агрегирует в себе другие, локи на них не берутся.

Здравствуйте, Аноним, Вы писали:

А>Вопрос: не слишком ли извращенно я решил эту задачу?
Слишком.

notifier — лучше с большой буквы

Здравствуйте, bolshik, Вы писали:

B>Когда ты указываешь synchronized(object), ты пытаешься взять лок только на указанный объект, т.е. если объект агрегирует в себе другие, локи на них не берутся.
Спасибо.

Здравствуйте, Георгий Вячеславович, Вы писали:
А>>Вопрос: не слишком ли извращенно я решил эту задачу?
ГВ>Слишком.

ГВ>notifier — лучше с большой буквы

"Слишком" заключается в том, что notifier с маленькой буквы или что-то другое имелось в виду?

Ладно поставлю вопрос по-другому. Можете описать в 2-3-х словах какая схема решения вам приходит в голову при прочтении этого упражнения?

Здравствуйте, Аноним, Вы писали:

А> Напишите программу, которая каждую секунду отображает на экране данные о времени, прошедшем от начала сессии, а другой ее поток выводит сообщение каждые 5 секунд.

А>Вопрос: не слишком ли извращенно я решил эту задачу?

Балда, кто же так пишет? Надо Spring + Quartz юзать!

PS:

Здравствуйте, Аноним, Вы писали:

А>Здравствуйте.
А>В книжке по джаве, после описания функций wait/notify/notifyAll/sleep и др. идет такое упражнение.
А>

А>Напишите программу, которая каждую секунду отображает на экране данные о времени, прошедшем от начала сессии, а другой ее поток выводит сообщение каждые 5 секунд. Предусмотрите возможность ежесекундного оповещения потока, воспроизводящего сообщение, потоком, отсчитывающим время. Не внося изменений в код потока-"хронометра" , добавьте еще один поток, который выводит на экран другое сообщение каждые 7 секунд.

А>Вопрос: не слишком ли извращенно я решил эту задачу? В книге методы wait/notify* применялись только по отношению к this.

Насчет применения wait/notify не только по отношению к this, можешь не беспокоиться -- это вполне легально.

Однако, а натравил findbugs твой код и выявил такие проблемы (потенциальные, для данного решения некритичные):

• SWL: Exercise.timer() calls Thread.sleep() with a lock held
  This method calls Thread.sleep() with a lock held. This may result in very poor performance and scalability, or a deadlock, since other threads may be waiting to acquire the lock. It is a much better idea to call wait() on the lock, which releases the lock and allows other threads to run.
  

  А>    synchronized public void timer()
  А>    {
  А>        . . .
  А>            while(true)
  А>            {
  А>                Thread.sleep(1000);
  А>        . . .
  А>            }
  А>        . . .
  А>    }

  
  
  
• UW: Unconditional wait in notifier.run()
  This method contains a call to java.lang.Object.wait() which is not guarded by conditional control flow. If the condition that the method intends to wait for has already happened, the thread could wait indefinitely.
  

  А>                synchronized(event) // *
  А>                {
  А>                    event.wait();
  А>                }

Первая проблема решается очень легко, надо просто удалить совершенно ненужный модификатор synchronized.

Вторая проблема требует особого рассмотрения.
Допустим у нас есть два потока. Один ждет какого-то события, а другой оповещает первый о неком событии. При этом первый содержит такой код

synchronized(event) {
    event.wait();
}

а второй

synchronized(event) {
    event.notify();
}

При этом события могут развиваться по следующим сценариям:
1. Первый поток зависает на wait, а затем второй делает notify. В этом случае все работает как положено.
2. Второй поток делает notify, а лишь затем первый зависает на wait. В этом случае первый поток "пропускает" событие о котором его оповещает второй поток и может заснуть очень надолго. Чтобы этого не произошло первый поток должен прежде чем заснуть на wait проверить, не произошло ли уже событие, которое он собирается ждать. Это может выглядеть так:

synchronized (event) {
    if (!event.happened)
        event.wait();
}

synchronized (event) {
    event.happened = true;
    event.notify();
}

Теперь wait "защишен условием". Однако с учетом spurious wakeup этого мало. Дело в том, что поток заснувший на wait может проснуться сам по себе без видимой на то причины, поэтому wait нужно защишать не if'ом, а while'ом.

synchronized (event) {
    while (!event.happened)
        event.wait();
}

Здравствуйте, slskor, Вы писали:

S>Здравствуйте, Аноним, Вы писали:

А>> Напишите программу, которая каждую секунду отображает на экране данные о времени, прошедшем от начала сессии, а другой ее поток выводит сообщение каждые 5 секунд.

А>>Вопрос: не слишком ли извращенно я решил эту задачу?

S>Балда, кто же так пишет? Надо Spring + Quartz юзать!

S>PS:

S>>Сейчас Spring пихают даже туда, где он не особо-то и нужен.
...
B>А без IoC можно и в 3х классах запутаться с инициализацией

Уважаемый, Blazkowicz, вы поменяли свое мировоззрение? C чем связано выраженное несогласие? Надо было не Quartz, а штатный таймер брать?

Здравствуйте, slskor, Вы писали:

S>Уважаемый, Blazkowicz, вы поменяли свое мировоззрение? C чем связано выраженное несогласие? Надо было не Quartz, а штатный таймер брать?

Не смотря на смайлик, грубо и не в тему. У человека академическя разработка а не промышленая.

Здравствуйте, dshe, Вы писали:

D>Теперь wait "защишен условием". Однако с учетом spurious wakeup этого мало. Дело в том, что поток заснувший на wait может проснуться сам по себе без видимой на то причины, поэтому wait нужно защишать не if'ом, а while'ом.

Интересно. А с чем связана проблема возникновения spurious wakeup, которое will rarely occur in practice?

Здравствуйте, slskor, Вы писали:

S>Интересно. А с чем связана проблема возникновения spurious wakeup, которое will rarely occur in practice?

здесь

Здравствуйте, Аноним, Вы писали:

А>Здравствуйте.
А>...

Я бы решил так:

public class CCC {

    public static void main(String[] args) throws IOException, InterruptedException {
        final MyTimer timer = new MyTimer();
        timer.start();

        Thread t1 = new Thread() {
            public void run() {
                TimeListener listener = new TimeListener() {
                    public int getFrequency() {
                        return 5;
                    }

                    public void onTimeout() {
                        System.out.println("#5: hello");
                    }
                };
                timer.addListener(listener);
                try {
                    Thread.currentThread().join();
                } catch (InterruptedException ignore) {
                }
            }
        };
        t1.setDaemon(true);
        t1.start();

        Thread t2 = new Thread() {
            public void run() {
                TimeListener listener = new TimeListener() {
                    public int getFrequency() {
                        return 7;
                    }

                    public void onTimeout() {
                        System.out.println("#7: hello");
                    }
                };
                timer.addListener(listener);
                try {
                    Thread.currentThread().join();
                } catch (InterruptedException ignore) {
                }
            }
        };
        t2.setDaemon(true);
        t2.start();

        Thread.currentThread().join();
    }
}

interface TimeListener {
    /** Means that onTimeout() should be invoked one time in returned period in seconds. */
    int getFrequency();
    void onTimeout();
}

class MyTimer {

    private static class DefaultTimeListener implements TimeListener {

        private long start;

        public int getFrequency() {
            return 1;
        }

        public void onTimeout() {
            System.out.println((System.currentTimeMillis() - start) + " milliseconds passed");
        }

        public void prepare() {
            start = System.currentTimeMillis();
        }
    }

    private final Map<TimeListener, Integer>/*<Listener, counter>*/ listeners = new HashMap<TimeListener, Integer>();
    private final DefaultTimeListener defaultTimeListener = new DefaultTimeListener();
    private volatile boolean keepRunning;
    private boolean started;

    public MyTimer() {
        addListener(defaultTimeListener);
    }

    public void start() {
        if (started) {
            throw new IllegalStateException("Timer is already started");
        }
        started = true;
        Thread thread = new Thread() {
            public void run() {
                System.out.println("timer started");
                defaultTimeListener.prepare();
                keepRunning = true;
                resetListeners();
                while(keepRunning) {
                    synchronized(listeners) {
                        for (TimeListener listener : listeners.keySet()) {
                            if (listener.getFrequency() <= listeners.get(listener)) {
                                try {
                                    listener.onTimeout();
                                } catch (Exception ignore) {
                                }
                                listeners.put(listener, 1);
                            } else {
                                listeners.put(listener, listeners.get(listener) + 1);
                            }
                        }
                    }
                    try {
                        sleep(1000);
                    } catch (InterruptedException e) {
                        MyTimer.this.stop();
                    }
                }
                started = false;
                System.out.println("timer stopped");
            }
        };
        thread.setDaemon(true);
        thread.start();
    }

    public void stop() {
        keepRunning = false;
    }

    public void addListener(TimeListener listener) {
        if (null == listener) {
            throw new IllegalArgumentException("Can't process MyTimer.addListener() for "
                                                       + "the null listener reference.");
        }
        if (0 >= listener.getFrequency()) {
            throw new IllegalArgumentException("Can't process MyTimer.addListener() for the listener={"
                                               + listener + "}. Listeners frequency must be positive, "
                                               + "current is " + listener.getFrequency());
        }
        synchronized(listeners) {
            listeners.put(listener, 0);
        }
    }

    public void removeListener(TimeListener listener) {
        if (null == listener) {
            throw new IllegalArgumentException("Can't process MyTimer.removeListener() for "
                                                       + "the null listener reference.");
        }
        synchronized(listeners) {
            if (null == listeners.remove(listener)) {
                throw new IllegalArgumentException("Can't remove unregistered listener={" + listener + "}");
            }
        }
    }

    private void resetListeners() {
        synchronized(listeners) {
            for (TimeListener listener : listeners.keySet()) {
                listeners.put(listener, 0);
            }
        }
    }
}

Здравствуйте, dshe

Спасибо!

Здравствуйте, slskor, Вы писали:

S>Интересно. А с чем связана проблема возникновения spurious wakeup, которое will rarely occur in practice?

По-моему грабли уходят корнями в операционные системы.

Здравствуйте, slskor, Вы писали:

S>Здравствуйте, dshe, Вы писали:

D>>Теперь wait "защишен условием". Однако с учетом spurious wakeup этого мало. Дело в том, что поток заснувший на wait может проснуться сам по себе без видимой на то причины, поэтому wait нужно защишать не if'ом, а while'ом.

S>Интересно. А с чем связана проблема возникновения spurious wakeup, которое will rarely occur in practice?

На мой взгляд, spurious wakeup в java вызван наличием этого явления в pthreads. На nix'ах многие системные вызовы при получении какого-либо сигнала (SIGALRM например) завершаются с кодом ошибки EINTR. А библиотечные оболочки над системными вызовами (open или creat например) в этом случае просто заново перезапускают системный вызов и все работает как ни в чем небывало. Однако для системного вызова pthread_wait перезапуск не делается поскольку возможны ситуации, когда поток был прерван сигналом и оповещен из другого потока pthread_signal или pthread_broadcast еще до того, как был перезапущен. В принципе, можно было бы сделать полностью корректную реализацию без spurious wakeup'ов в ущерб, скажем, производительности. Но дизайнеры pthreads пошли по другому пути и решили просто задекларировать возможность spurious wakeup'ов. Тем более, они практически без каких-либо существенных накладных расходов обрабатываются в прикладном коде.

На Windows платформе, если такое и происходит, причин я не знаю.

Здравствуйте, Аноним, Вы писали:

А>"Слишком" заключается в том, что notifier с маленькой буквы или что-то другое имелось в виду?

"Слишком" можно читать как "ниасилил"

А>Ладно поставлю вопрос по-другому. Можете описать в 2-3-х словах какая схема решения вам приходит в голову при прочтении этого упражнения?

Очень просто:

Метод sleep() гарантирует, что ваш поток будет спать как минимум столько, сколько указано в аргументе. Поэтому, "написать программу, которая каждую секунду выводит ..." и "предусмотреть возможность ежесекундного ..." строго говоря — невозможно. А решать некоректно поставленные задачи не интересно. Так что мне импонирует схема, согласно которой книга сжигается

Если игнорировать эти факты, то про ошибки вам уже сказали. Почти со всем согласен.

Вот это

Автор: bolshik
Дата: 27.10.06

"слишком".

Здравствуйте, bolshik, Вы писали:

B>Здравствуйте, Аноним, Вы писали:

B>Я бы решил так:
...

Ага. А еще можно EJB использовать в качестве таймера.

Кроме того, почему в коде вывод в стандартный поток вывода, когда уместнее было бы использовать логгирование (параметризировать классом логгера ваши классы, например) и предоставить реализацию логгера по умолчанию с выводом всего в поток вывода?


B>interface TimeListener {
B> /** Means that onTimeout() should be invoked one time in returned period in seconds. */
B> int getFrequency();
B> void onTimeout();
B>}

Т.е. вместо того, чтобы продумать дизайн, вы выносите часть контракта в комментарии?

Очень я сомневаюсь, что во времена, когда вы читали подобные книги, вы бы написали такой код.

Здравствуйте, Георгий Вячеславович, Вы писали:

ГВ>...

ГВ>Ага. А еще можно EJB использовать в качестве таймера.

Не понял связи.


ГВ>Кроме того, почему в коде вывод в стандартный поток вывода, когда уместнее было бы использовать логгирование (параметризировать классом логгера ваши классы, например) и предоставить реализацию логгера по умолчанию с выводом всего в поток вывода?

Этого не требовалось в условии задания.


B>>interface TimeListener {
B>> /** Means that onTimeout() should be invoked one time in returned period in seconds. */
B>> int getFrequency();
B>> void onTimeout();
B>>}

ГВ>Т.е. вместо того, чтобы продумать дизайн, вы выносите часть контракта в комментарии?

Т.е. джавадок на метод интерфейса это просто 'комментарий'? Дизайн чего здесь не продуман?


ГВ>Очень я сомневаюсь, что во времена, когда вы читали подобные книги, вы бы написали такой код.

Если бы мне во времена читания подобных книг показали подобный код, сказал бы большое спасибо.

Здравствуйте, bolshik, Вы писали:

B>Не понял связи.

Это у меня юмор такой несмешной. Суть моего поста сводиться к тому, что задачу можно решить гораздо проще.

Здравствуйте, Георгий Вячеславович, Вы писали:

ГВ>Это у меня юмор такой несмешной. Суть моего поста сводиться к тому, что задачу можно решить гораздо проще.

Можно проще. А можно правильнее, хотя правильность — понятие относительное.