Dave Z
Dave Z

Reputation: 159

Why doesnt this Java loop in a thread work?

I'm coding a radio in java and am using a thread to of course play the stream in. I have a while loop in the run method of the thread which checks if a variable called shouldPlay is true, and if it is it runs the player. Then, I have a pause() method which sets the variable to false and naturally I want it to not run the player when the variable is false. I've created a small example of how I've set it up below:

class audiostream extends Thread {

    private boolean shouldPlay = true;

    @Override
    public void run() {
        while(true)
        { 
            if(shouldPlay)
            {
                shouldPlay = false;
                System.out.print("test");
            }
        }
    }

    public void pause() {
        shouldPlay = false;
    }

    public void play() {
        shouldPlay = true;
    }

}

What I want to happen is when the thread first runs, it should print "test" (which it does fine). Then when you run pause() it should set shouldPlay to false and not allow "test" to be printed out. But then when you call play() it should set shouldPlay to true then print out test again. The problem is it's not doing this.

About 1/100th of the time it will though. Am I right to assume the problem here is even though I call play() and pause() from another thread as not to "hang" the parent thread, that my child thread is hanging on that while loop and when calling play() or pause() from the parent the variables don't change value because of that loop? If that's the case I don't know what I should do!

Upvotes: 2

Views: 211

Answers (4)

meriton
meriton

Reputation: 70574

The Java Memory Model only guarantees that threads will see changes made by other threads if they synchronize-with those threads. The Java Language Specification defines synchronized-with as follows:

Synchronization actions induce the synchronized-with relation on actions, defined as follows:

  • An unlock action on monitor m synchronizes-with all subsequent lock actions on m (where subsequent is defined according to the synchronization order).
  • A write to a volatile variable (§8.3.1.4) v synchronizes-with all subsequent reads of v by any thread (where subsequent is defined according to the synchronization order).
  • An action that starts a thread synchronizes-with the first action in the thread it starts.
  • The write of the default value (zero, false or null) to each variable synchronizes-with the first action in every thread. Although it may seem a little strange to write a default value to a variable before the object containing the variable is allocated, conceptually every object is created at the start of the program with its default initialized values.
  • The final action in a thread T1 synchronizes-with any action in another thread T2 that detects that T1 has terminated. T2 may accomplish this by calling T1.isAlive() or T1.join().
  • If thread T1 interrupts thread T2, the interrupt by T1 synchronizes-with any point where any other thread (including T2) determines that T2 has been interrupted (by having an InterruptedException thrown or by invoking Thread.interrupted or Thread.isInterrupted).

In your example, the thread writing shouldPlay and the thread reading it do none of that, thereby failing to establish synchronized-with. Therefore it is unspecified if and when the reading thread will notice the new value.

In your case, declaring shouldPlay volatile is the easiest way to establish that synchronization.

Upvotes: 1

Scorpion
Scorpion

Reputation: 3986

You can make use of AtomicBoolean as an alternative.

Upvotes: 1

KV Prajapati
KV Prajapati

Reputation: 94643

shouldPlay should be volatile and please remove - shouldPlay = false;,

if(shouldPlay)
 {
  //  shouldPlay = false;
      System.out.print("test");
 }

Upvotes: 1

irreputable
irreputable

Reputation: 45443

add 'volatile' will fix it

private volatile boolean shouldPlay = true;

Upvotes: 4

Related Questions