Why does this break statement break not work?

Question

I have the following code:

public void post(String message) {
    final String mess = message;
    (new Thread() {
        public void run() {
            while (true) {
                try {
                    if (status.equals("serviceResolved")) {
                        output.println(mess);
                        Game.log.fine("The following message was successfully sent: " + mess);
                        break;
                    } else {
                        try {Thread.sleep(1000);} catch (InterruptedException ie) {}
                    }
                } catch (NullPointerException e) {
                    try {Thread.sleep(1000);} catch (InterruptedException ie) {}
                }
            }
        }
    }).start();
}

In my log file I find a lot of lines like this:

The following message was successfully sent: blablabla
The following message was successfully sent: blablabla
The following message was successfully sent: blablabla
The following message was successfully sent: blablabla

And my program is not responding.

It seems to me that the break command does not work. What can be a possible reason for that.

The interesting thing is that it happens not all the time. Sometimes my program works fine, sometimes the above described problem happens.

Answer 1

Could it be that this line succeeds:

output.println(mess);

but this line is throwing a null pointer exception:

Game.log.fine(...

In this case you'll see the output on the console, but the break statement is never reached. Is Game.log perhaps null?

Answer 2

You are starting a new thread every time you call the post method. I thing the method is OK but caller program isn't.

Answer 3

What exactly does Game.log.fine do? Could it be that it throws a NullPtrException after output, or could it be that you call the post-method several times?

Remove the catch of the NullPointerException, this is bad style (occurrence of a NullPointerException is always a programming error) and add some more log-messages in the method (or use a debugger).

Answer 4

Are you sure you want to keep going on a NullPointerException? If you get one inside the loop, you're likely to wait forever.

If you're sure that status will always eventually be "serviceResolved", then put a try...finally inside the if statement so that if something fails, the loop still exits:

if (status.equals("serviceResolved")) {
    // No matter what happens next, we have to bail
    try {
        output.println(mess);
        Game.log.fine("The following message was successfully sent: " + mess);
    } finally {
        break;
    }
} else {
    try {Thread.sleep(1000);} catch (InterruptedException ie) {}
}

Answer 5

You're assuming that the break statement isn't working, but it could be that your post method is being called repeatedly. Try putting another log statement at the beginning of the method to see how often it is being called. Also, put a log statement after the while loop but before the end of the run method to verify that the break actually broke out of the loop.

I also agree with the other posters that catching NullPointerException is a code smell. You should check your variables for null first.