Boolean not correctly updating unless you print it

Question

So I am making this java applet and I just want to first make sure the key input works correctly before making it more complex. I have no idea why, but when you remove the "System.out.print(needUpdating);" it does not correctly moves the rectangle according to the key input. Could anyone tell me why and how to fix it? This is a total mystery to me.

import java.applet.Applet;
import java.awt.Graphics;
import java.awt.Rectangle;
import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;    
import javax.swing.JApplet;    

public class firstApplet extends JApplet implements KeyListener, Runnable {
    final int MOVEAMOUNT = 1;       
    boolean needUpdating;       
    int x,y,dx,dy;      
    Thread runner = null;

    public void init() {
        this.setFocusable(true);            
        needUpdating = false;           
        this.requestFocusInWindow();            
        x=0;            
        y=0;            
        dx=0;           
        dy=0;           
        addKeyListener(this);
    }

    public void stop() {            
    }

    public void start() {
        runner = new Thread(this);          
        runner.start();
    }

    public void paint(Graphics g) {
        System.out.println("x= "+x+" y = "+y);          
        g.drawRect( x, y, 100, 15 );
    }

    @Override
    public void keyPressed(KeyEvent e) {                        
        int key = e.getKeyCode();           
        if(key==KeyEvent.VK_UP) {
            System.out.println("up");
            dy=MOVEAMOUNT;
        }
        else if (key==KeyEvent.VK_DOWN) {
            dy=-MOVEAMOUNT;
        }
        else if (key==KeyEvent.VK_LEFT) {
            dx=-MOVEAMOUNT;
        }
        else if (key==KeyEvent.VK_RIGHT) {
            dx=MOVEAMOUNT;
        }
        // TODO Auto-generated method stub          
        needUpdating = true;            
        System.out.println("needUpdating listening = " +needUpdating);
    }

    @Override
    public void keyReleased(KeyEvent e) {
        // TODO Auto-generated method stub
        dx=0;
        dy=0;
    }

    @Override
    public void keyTyped(KeyEvent e) {          
    }

    public void processMovement() {
        System.out.println("processing");
        x+=dx;
        y+=dy;
    }

    @Override
    public void run() {
        this.addKeyListener(this);
        while(true) {
            System.out.print(needUpdating);             
            if(needUpdating) {
                processMovement();                  
                repaint();                  
                needUpdating=false;
            }           
        }           
    }
}

Try to make `needUpdating` field `volatile`: `volatile boolean needUpdating;` — Andremoniy, Jul 02 '13 at 13:45

score 5 · Answer 1 · answered Jul 02 '13 at 13:45

5

Your code is incorrectly synchronized. Better said, it isn't synchronized at all. However, System.out.println is a synchronized method, and on today's typical CPU architectures, entering a synchronized block happens to be implemented as a memory barrier at the native code level. This has the effect of making the changes to your boolean visible to other threads.

Conclusion: correctly synchronize your code and the "magical" behavior will disappear.

answered Jul 02 '13 at 13:45

Marko Topolnik

195,646
29
319
436

I fixed these bug and find out the rectangle goes up when you press down arrow. – johnchen902 Jul 02 '13 at 13:53
1

That's not suprising since you decrease `y` in that case, and the coordinate system's origin is the upper left corner. – Marko Topolnik Jul 02 '13 at 13:55

Kayaman · Answer 2 · 2013-07-02T14:00:16.013

There are serious issues with this code. First of all you are implementing Runnable for no good reason. Second of all, your run() method is a big busy-wait loop. And thirdly of course the lack of synchronization in your needsUpdating variable.

Instead of using a thread-unsafe needsUpdating variable, you should perform the required operations in the listener methods, and the thread safety issue will disappear since you'll be in the Event Dispatch Thread.

Andremoniy · Answer 3 · 2013-07-02T13:55:18.313

1

You have to make your field needUpdating as volatile:

volatile boolean needUpdating;

This behaviour is defined by infinite loop inside run() method: JVM caches value of needUpdating field.

UPD: I just checked your code: it works fine with volatile modifier on field needUpdating, so my answer is the solution.

UPD2: For clarifying this problem, look at example in Chapter 17 of JLS7: 17.3. Sleep and Yield

edited Jul 02 '13 at 13:55

answered Jul 02 '13 at 13:48

Andremoniy

34,031
20
135
241

Boolean not correctly updating unless you print it

3 Answers3