Code not executed without a print statement

Question

i've been making a countdown program, and i came up with this.

package main;

import java.awt.FlowLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.io.File;
import java.io.IOException;
import java.net.MalformedURLException;

import javax.sound.sampled.AudioInputStream;
import javax.sound.sampled.AudioSystem;
import javax.sound.sampled.Clip;
import javax.sound.sampled.DataLine;
import javax.sound.sampled.LineUnavailableException;
import javax.sound.sampled.UnsupportedAudioFileException;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JTextField;

public class Gatoo extends JFrame implements ActionListener {
    private int sec, min, secTot, since = 999;
    private long lastTime;

    private JTextField mm = new JTextField(2), ss = new JTextField(2);
    private JLabel minLab = new JLabel("Minutes:"), secLab = new JLabel(
            "Seconds:");
    private JButton start = new JButton("Start");

    private Clip done;
    private boolean started = false;

    private static final long serialVersionUID = 4277921337939922028L;

    public static void main(String[] args) {
        Gatoo cake = new Gatoo("Title");
        cake.pack();
        cake.setSize(800, 600);
        cake.setLocationRelativeTo(null);
        cake.setDefaultCloseOperation(3);
        cake.setVisible(true);
        cake.run();
    }

    public Gatoo(String s) {
        super(s);
        setLayout(new FlowLayout());

        start.addActionListener(this);

        add(minLab);
        add(mm);
        add(secLab);
        add(ss);
        add(start);
    }

    @Override
    public void actionPerformed(ActionEvent e) {
        started = true;
    }

    public void play(File file) throws MalformedURLException,
            UnsupportedAudioFileException, IOException,
            LineUnavailableException {
        AudioInputStream ais = AudioSystem.getAudioInputStream(new File(
                "lib/done.wav"));
        DataLine.Info info = new DataLine.Info(Clip.class, ais.getFormat());
        done = (Clip) AudioSystem.getLine(info);
        done.open(ais);
        done.start();
    }

    public void run() {
        while (true) {
            System.out.print("");// needed?
            if (started) {
                try {
                    min = Integer.parseInt(mm.getText());
                    sec = Integer.parseInt(ss.getText());
                    secTot = (min * 60) + sec;
                    lastTime = System.currentTimeMillis();
                    while (secTot > 0) {
                        since = (int) (System.currentTimeMillis() - lastTime);
                        if (since > 998) {
                            lastTime = System.currentTimeMillis();
                            secTot--;
                        }
                    }

                    play(new File("done.wav"));

                } catch (NumberFormatException exception) {
                    System.out.println("Minutes and seconds must be numbers.");
                    return;
                } catch (Exception exception) {
                    exception.printStackTrace();
                }
                started = false;
            }
        }
    }
}

In the while loop at the end the countdown code doesn't execute without a print / println statement inside. How come? The program works perfectly fine with the print statement though.

I do not believe you, to be honest. I think that your tests were false in some way. — Martijn Courteaux, Dec 26 '13 at 14:31
Run condition. The `println` just delays the code so another condition (probably setting `started` to true) has time to be set by another thread. Revise the timeline of your operations.. — SJuan76, Dec 26 '13 at 14:31
@SJuan76: That was what I though as wel, but he put his code in a while true block (auch!) — Martijn Courteaux, Dec 26 '13 at 14:32
All else aside, why not use a Swing Timer instead of trying to implement your own? — Paul Samsotha, Dec 26 '13 at 14:36
@user30902987: Try to add the `volatile` keyword to your `boolean started` declaration. — Martijn Courteaux, Dec 26 '13 at 14:36
He specifically sets 'started' value to false if it is not true on the first pass so it will never become true, so the while true just continues running. — ialexander, Dec 26 '13 at 14:40

Marko Topolnik · Accepted Answer · 2013-12-26T15:12:31.727

14

First and foremost, your program is thread-unsafe because boolean started is a shared variable, but it is neither volatile nor accessed within synchronized blocks.

Now, accidentally, PrintStream#print is a synchronized method and, on any actual architecture, entering and exiting a synchronized block is implemented using memory barrier CPU instructions, which cause a complete synchronization between the thread-local state and main memory.

Therefore, by pure accident, adding the print call allows the setting of started flag by one thread (the EDT) to be visible by another (the main thread).

edited Dec 26 '13 at 15:12

answered Dec 26 '13 at 14:43

Marko Topolnik

195,646
29
319
436

+1 for actually answering correctly. – Martijn Courteaux Dec 26 '13 at 14:49

score 4 · Answer 2 · edited Dec 26 '13 at 15:30

4

You have poor design for Swing application.

Don't use while(true) loop in your run() method. Read more about Concurency in Swing.
Call events with help of Listeners(ActionListener e.g.) instead of flags(started here).
Instead of counting time use Swing Timer.

Change your run() method like next:

public void run() {
      min = Integer.parseInt(mm.getText());
      sec = Integer.parseInt(ss.getText());
      secTot = (min * 60) + sec;
      Timer timer = new Timer(1000*secTot, new ActionListener() {

        @Override
        public void actionPerformed(ActionEvent e) {
              try {
                play(new File("done.wav"));
            } catch (Exception e1) {
                e1.printStackTrace();
            }
        }
    });
      timer.start();
}

actionPerformed() method :

@Override
public void actionPerformed(ActionEvent e) {
    run();
}

and remove cake.run() in main method.

edited Dec 26 '13 at 15:30

Andrew Thompson

168,117
40
217
433

answered Dec 26 '13 at 14:36

alex2410

10,904
3
25
41

1

He's actually not blocking the EDT, since he never switches to the EDT in main. – Boann Dec 26 '13 at 14:37
-1: He is indeed not blocking the EDT. There is just a lot of load on the main thread. – Martijn Courteaux Dec 26 '13 at 14:39
Yeah, now I noticed that fact. – alex2410 Dec 26 '13 at 14:41
I turned the list into an ordered list. Please check it still meets your intended meaning (that I did not stuff up). – Andrew Thompson Dec 26 '13 at 15:30
1) I used a while(true) loop because i wanted the countdown function to be available more than once. 2) I used a flag because the "Start" button was stuck when i executed the code inside the ActionListener. 3) That was actually a good idea, forgot about it when i wrote the code. – user3092987 Dec 26 '13 at 15:35

Martijn Courteaux · Answer 3 · 2013-12-26T15:02:40.950

2

Look, I made a SSCCE reproducing this behavior. It is a really good question.

public class ThreadRacing implements Runnable
{
    public boolean started = false;

    public static void main(String[] args)
    {
        new ThreadRacing().test();
    }

    public void test()
    {
        new Thread(this).start();
        try
        {
            Thread.sleep(1000);
        } catch (Exception e)
        {

        }
        started = true;
        System.out.println("I did my job");
    }

    @Override
    public void run()
    {
        while (true)
        {
            //System.out.print("");
            if (started)
            {
                System.out.println("I started!!");
            }
        }
    }

}

This prints: "I did my job". Nothing more. Adding a volatile keyword actually fixes the problem.

To me, it looks like the second Thread gets not notified about the update to started because he is too bussy.

edited Dec 26 '13 at 15:02

answered Dec 26 '13 at 14:48

Martijn Courteaux

67,591
47
198
287

1

After one second, the JIT compiler will have gotten to the `run` method and recompiled it so that the complete `if` construct is eliminated as dead code. It has the right to do so because `started` is non-volatile and cannot be changed within the `while(true)` loop. So this doesn't really prove anything about threads being busy---if that was true, then making the variable `volatile` wouldn't make them any less busy. – Marko Topolnik Dec 26 '13 at 14:52
1

What do you mean? *After one second, the JIT compiler will have gotten to the run method and recompiled it*. Are you sure you looked right at my code? – Martijn Courteaux Dec 26 '13 at 14:54
The `run` method executes for one second before the `started` flag is set to `true`. That gives the loop in `run` enough runtime to reach the (default) JIT threshold of 10,000 iterations, after which the compiled code is substituted for the interpreted original (using On-Stack Replacement). – Marko Topolnik Dec 26 '13 at 14:55
What!? Do you mean that if I write a loop with an if that results 12000 times into false, it will get rid of the if? I just ran the same application in the debugger, and when I pause the process make 1 step and run again it gets into the if. Does this mean that the behavior of the deadcode elimination process is different when debugging? – Martijn Courteaux Dec 26 '13 at 14:59
1

AFAIK, there's no compilation whatsoever when the JVM is in debugging mode. Otherwise stepping through code, breakpoints, and pretty much anything else would be impossible. – Marko Topolnik Dec 26 '13 at 15:01
1

But note that in this particular example, you can set the compilation threshold to just 1 and the JIT compiler will *still* be allowed to optimize away the `if` because the whole `run` method involves no inter-thread operations at all (whenever the `if` condition evaluates to `false`, that is). Therefore the compiler can pretend the thread executing `run` is the only thread around. – Marko Topolnik Dec 26 '13 at 15:02
Thank you very much. I learnt something today (unlike my chemistry I actually have to study for my finals). I'll read about it :) – Martijn Courteaux Dec 26 '13 at 15:04
1

It pays to study the Java Memory Model chapter of the JLS. Even better, read [the Java Memory Model paper](http://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.142.1179) by Jeremy Manson et al. The JLS version is imprecise at times, as it tries to be a more accessible (dumbed-down) read. – Marko Topolnik Dec 26 '13 at 15:09
Yes, indeed. The problem is that I wanted to study civil engineer with the option computer science. But in the first general year, you see everything, so you would be able to make your choice, but I definitely know what I want to study. I'm not sure if I made the right decision by choosing for civil engineer. Maybe I wanted to do plain IT. Not sure. – Martijn Courteaux Dec 26 '13 at 15:28

Boann · Answer 4 · 2013-12-26T14:58:37.357

-1

I would surmise that your busy-wait loop is hogging the CPU so severely it is unable to do anything. The print statement is causing just enough of a thread context switch that it is able to get other work done.

Edit: Okay, I did a little testing. I was able to reproduce OP's problem on the HotSpot Server VM. Using Thread.currentThread().setPriority(Thread.MIN_PRIORITY); did not fix it, so it is not a starvation issue. Setting the variable to volatile as @MartinCourteau, @MarkoTopolnik suggested, did fix it. That makes sense. I couldn't originally reproduce the problem on the HotSpot Client VM; apparently its optimizations are too weak for it to cache the started variable.

(Still, if the Java audio thread had a lower than normal thread priority and it were a single-CPU system, starvation was a plausible hypothesis.)

edited Dec 26 '13 at 14:58

answered Dec 26 '13 at 14:33

Boann

48,794
16
117
146

-1: Can you explain how this answer is actually valid if his code is in a while(true)? – Martijn Courteaux Dec 26 '13 at 14:34
1

@MartijnCourteaux That's exactly why it's valid. The while loop is starving the audio system. – Boann Dec 26 '13 at 14:38
You can't starve a thread that naïvely on a preemptively multitasked system. – Marko Topolnik Dec 26 '13 at 14:47

Code not executed without a print statement

4 Answers4

Linked

Related