Not seeing a static class variable changing from another class in the same package running in a separate thread

Question

I have a GUI class that, upon a button press, assigns a value to a private static class variable, -- let's call it strX.

The constructor of class GUI also instantiates a subclass (in the same package). This subclass implements Runnable, and starts a thread. There is an infinite loop in the thread which keeps querying the value of strX until it is no longer null. The querying is done by calling a getter method in the GUI class.

I notice that even after I have pressed the button and set strX to some non-null value, the querying in the subclass produces only null.

I am new to Java, and I am beginning to wonder if my understanding is flawed.

I've read Java: cannot access static variable from a different class in the same package and its 4 answers, which address a situation having to do with a clash of variable names. My situation is certainly not this. All I am trying to do is to use a getter method to obtain a class variable from a subclass.

Please let me know if any aspect of my question needs clarification. I would post the code, but it is long and will probably add to the confusion.

Follow-up

Okay, first of all, I'd like to thank Darren Gilroy for his detailed answer and code snippet. That cleared up some confusion in my mind, but I still had problems. So I decided to create a simple stripped-down version of my code for clarity and started testing it. This simple GUI class has two buttons, one called 'Only Instantiate', and the other called 'Set Value and Instantiate'.

1. When you push the 'Only Instantiate', it creates an instance of another class called PeekAtGUI. Inside PeekAtGUI, a thread is started. The thread invokes a static getter method of GUI to obtain the current value of a static class variable strX in GUI.

2. When you push the 'Set value and Instantiate' button, it first sets the value of strX to a new value, and then instantiates PeekAtGUI.

Please note that as I am debugging, I comment out one or other of these instantiations.

Here is the bottomline- When I try to instantiate first, and then push the other button to set the new value, PeekAtGUI doesn't seem to get the updated value. However, if I instantiate and set value with a single button push, then it all magically seems to work.

Not sure what is going on, and any clarification will be really appreciated. Thanks again.

Here is the code-

Class GUI

package intercom;

import java.awt.EventQueue;

import javax.swing.JFrame;
import javax.swing.JPanel;
import java.awt.BorderLayout;
import javax.swing.JButton;
import java.awt.event.ActionListener;
import java.awt.event.ActionEvent;

public class GUI {

    private JFrame frame;
    private static volatile String strX = "";

    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
            public void run() {
                try {
                    GUI window = new GUI();
                    window.frame.setVisible(true);
                } catch (Exception e) {
                    e.printStackTrace();
                }
            }
        });
    }

/**
 * Create the application.
 */
    public GUI() {
        initialize();
    }

/**
 * Initialize the contents of the frame.
 */
    private void initialize() {
        frame = new JFrame();
        frame.setBounds(100, 100, 450, 300);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        JPanel panel = new JPanel();
        frame.getContentPane().add(panel, BorderLayout.CENTER);
        panel.setLayout(null);

        JButton btnSet = new JButton("Set Value and Instantiate");
        btnSet.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                setVal(strX+"+");
                new PeekAtGUI();  // When PeekAtGUI instantiated here, it is able to get the new strX
            }
        });
        btnSet.setBounds(106, 94, 224, 23);
        panel.add(btnSet);

        JButton btnStart = new JButton("Only Instantiate");
        btnStart.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                //new PeekAtGUI();  // When PeekAtGUI is instantiated here, it is NOT able to get the new strX
            }
        });
        btnStart.setBounds(106, 39, 224, 23);
        panel.add(btnStart);
    }

    public static synchronized void setVal(String str){
        strX = str;
        GUI.class.notifyAll();
    }
    public static synchronized String getVal(){
        return strX;
    }
}

Class PeekAtGUI

package intercom;

public class PeekAtGUI implements Runnable{

    private static String msg = "";
    private boolean done = false;
    public PeekAtGUI() {
        Thread r = new Thread(this, "PeekAtGUI Thread");
        r.start();
    }



    public void run() {
        while (done == false){      
            try {getMsg();}catch (InterruptedException e) {e.printStackTrace();}
            System.out.println("Reporting value of strX from PeekAtGUI: " + msg);
            }
    }

    private static synchronized void getMsg() throws InterruptedException{
        while((msg==GUI.getVal())){PeekAtGUI.class.wait();}
        msg = GUI.getVal();
    }
}

Answer 1

Before you get too far into trying to share data across threads, you need to spend a bit of time learning about how Java treats various types of data when it comes to threading:

Even if you access a static value through multiple threads, each thread can have its local cached copy! To avoid this you can declare the variable as static volatile and this will force the thread to read each time the global value.

However, volatile is not a substitute for proper synchronisation! For instance ...

From the sound of things, you're reading the thread's cached value every time - you may or may not ever see the change, and trying to make your program execute predictably is going to be a major headache. Read the rest of stivlo's answer, and consider a more sane design for communicating between threads.

Answer 2

In java, threads are free to have local copies of variables independent from the heap. This fact is independent of both static variables and package semantics.

When thinking about the java memory model, I like to think of each thread having its own copy of memory, and this copy is synchronized with global state only when you force it to.

This is not entirely accurate, but you'll never be wrong thinking this way and just absorbing that will help you understand how different the Java memory model is from C/C++.

To re-cap your situation:

1. You have a memory location set from one thread
2. You're reading that memory from another thread
3. Java does not guarantee this is the same memory location

In your case, commenters have suggested adding volatile to the variable. This is not the same as C++ volatile. I mean, not really the same. What's happening in java is that accesses to a volatile variable use memory barriers to implement a happens-before relationship between the reader and writer. A handy way is to think that a write to a volatile variable "publishes" the new value to all reader threads. A write to a non-volatile variable does not.

Adding this will fix the thread that's spinning on the value b/c it will see the change immediately. (More properly, it will see the change in reference held by the variable.)

However, spinning is an uncommon pattern in Java, because java has tons of easy to use locks sitting around (on every object!!) and reliable wait/notify implementations. So a java programmer might do this in your case:

class Foo {
  private static String value;
  // synchronized keyword on a static method locks Foo.class
  public static synchronized getValue() {
    while(value == null) Foo.class.wait(); // can wait only when you hold the lock
    return value;
  }
  public static synchronized setValue(String v) {
    value = v;
    Foo.class.notifyAll(); // can notify only when you hold the lock
  }
}

The "static synchronized" method is locking the monitor on Foo.class. This is actually not a bad introduction: http://docs.oracle.com/javase/tutorial/essential/concurrency/sync.html

Good luck.

Answer 3

In response to the follow up, let's look at this from your reader...

private static synchronized void getMsg() throws InterruptedException{
    while((msg==GUI.getVal())){PeekAtGUI.class.wait();}
    msg = GUI.getVal();
}

And this from your writer...

public static synchronized void setVal(String str){
    strX = str;
    GUI.class.notifyAll();
}

The issue that jumps out right away is that you're waiting on one object (PeekAtGUI.class), yet you're notifying on a different object (GUI.class). So you need to fix that by having your actors communicating through on the same object. All monitors are public in java so you can just switch one side. (Public monitors on every object is not a popular feature of Java!)

An aside: I think the effect of this would be to park the reading thread. Is that what you see?

A second thing is the use of == to compare things. Beware that this is reference (pointer?) equality and not value equality. I'm not sure the purpose of the value check there? Since getMsg() is already in a loop, I suspect you don't need to have another loop wrapping the wait. Unless there is something I'm missing, the following might work:

public void run() {
  while (done == false) { // 'done' should be volatile
    try {
      // you don't need to be in a synchronized block to read
      String msg = GUI.getVal(); 
      System.out.println("Reporting value of strX from PeekAtGUI: " + msg);
      // but you do need lock before you can wait. 
      // It's unusual to lock a class externally in this way, instead
      // one might provide a GUI.waitForNextVal() method.
      synchronized(GUI.class) {
        GUI.class.wait();
      }
    } catch (InterruptedException e) {e.printStackTrace();}
  }
}

It feels like you're getting closer. I'm a bit rusty working at this low level. We java programmers love our abstractions! http://www.ibm.com/developerworks/java/library/j-5things4/index.html?ca=drs-

Good luck again.