Private member in a thread getting changed from another thread

Question

My first question here. Java self taught, not an expert.

I am having a main application creating a manager thread. Manager will create worker threads. Main will pass on "Properties" as parameter to Manager. Manager will make some additions to the properties and pass it on as parameter to each Worker.

Problem:

After instantiating the worker objects when I make a change in the properties member that is part of Manager, the properties members that are part of worker threads are also getting changed. Since properties in Worker class is a private member, I thought it can only be changed within Worker class. Why is it getting changed when I make a change in the Manager? How can I avoid this issue?

Attached a simple version of the code here:

Main.java:

import java.util.Properties;

public class Main {
    public static void main(String arg[])
    {
        Properties properties = new Properties();
        properties.put("p1", "v1");
        Manager manager = new Manager(properties);
        manager.start();
    }
}

Manager.java:

import java.util.Properties;

public class Manager {
    private Properties properties;

    Manager(Properties argProperties)
    {
        properties = argProperties;
    }

    public void start()
    {
        properties.put("workerId", "worker1");
        spawnWorker();

        try {
            Thread.sleep(2000);
        }catch(InterruptedException ex){
            //do nothing
        }

        System.out.println("Manager going to change worker ID to 2");
        properties.put("workerId", "worker2");
        spawnWorker();

        try {
            Thread.sleep(2000);
        }catch(InterruptedException ex){
            //do nothing
        }

        Thread manager = new Thread("manager"){
            public void run() {
                System.out.println("Manager going to change worker ID to haha");
                while(true)
                {
                    try {
                        properties.put("workerId", "hahaha");
                        Thread.sleep(1000);
                    }catch(InterruptedException ex){
                        //do nothing
                    }
                }
            }
        };
        manager.start();
    }

    private void spawnWorker()
    {
        Worker worker = new Worker(properties);
        worker.start();
    }
}

Worker.java:

import java.util.Properties;

public class Worker {
    private final Properties properties;
    String workerId;

    Worker(Properties argProperties)
    {
        properties = argProperties;
        workerId = properties.getProperty("workerId");
    }

    public void start()
    {
        Thread worker = new Thread("worker-" + workerId){
            public void run() {
                while(true)
                {
                    try {
                        System.out.println(this.getName() + "<->" + properties.getProperty("workerId"));
                        Thread.sleep(1000);
                    }catch(InterruptedException ex){
                        //do nothing
                    }
                }
            }
        };
        worker.start();
    }
}

Output:

worker-worker1<->worker1
worker-worker1<->worker1
worker-worker1<->worker1
Manager going to change worker ID to 2
worker-worker2<->worker2
worker-worker1<->worker2
worker-worker2<->worker2
worker-worker1<->worker2
Manager going to change worker ID to haha
worker-worker2<->worker2
worker-worker1<->hahaha
worker-worker2<->hahaha

Passing a reference to an object to a method doesn't make a copy of the object. All your objects have a reference to a unique Properties object. I don't want to be harsh, but this is fundamental stuff, that you need to clearly understand before touching to threads, which are a much much more complex matter. — JB Nizet, Dec 12 '15 at 18:03
You need to understand the difference between a variable and an object. `private` is an attribute of the `properties` variable, but your problem happens because you have different variables that all refer to the same object. Like JB said, you should work through some basic Java lanugage tutorials before attempting to write multi-threaded code. — Solomon Slow, Dec 12 '15 at 18:12
Thanks to others for pointing out that I need to learn more. Accepted. Love to learn by mistakes :) _if only the bird with the sweetest voice sang the forest would be a quiet place... let all birds sing!_ — Joseph Alexander, Dec 12 '15 at 18:29

score 2 · Accepted Answer · edited May 23 '17 at 10:27

You are passing the same instance of Properties to every worker. Thus every worker can change it contents.

Prior to spawn a new worker you should copy the Properties and pass this copy to the worker.

// More background: Variable modifiers are for declaring the visibility of a variable. A private member can't be access from outer class. But it can be exposed via reference (as you are doing, by passing the reference via constructor) or a public getter. In consequence the exposed private members contents can be changed.

Details can be found here:

https://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html

and

Is Java "pass-by-reference" or "pass-by-value"?

While this is the issue and solution, it doesn't help the poster understand why `private` doesn't protect the contents of the Properties instance. — blm, Dec 12 '15 at 18:05

score 1 · Answer 2 · answered Dec 12 '15 at 18:18

Consider the following simpler case:

public class Car {

    private CarProperties properties;

    public Car(CarProperties properties) {
        this.properties = properties;
    }
}

public class CarProperties {

    private String make;
    private String year;

    public CarProperties(String make, String year)  {
        this.make = make;
        this.year = year;
    }

}

If you use these classes in the following way:

CarProperties properties = new CarProperties("Volvo", "1997");
Car car1 = new Car(properties);
Car car2 = new Car(properties);

Notice how there is only one instantiation of CarProperties, meaning only one object of this type is created. This means that car1 and car2 will share a reference to the same object and thus changing the fields of that object from wherever it has a reference will change the values of that object. If you instead create a new instance of CarProperties for every car, they can be changed independently.

private does not protect for this kind of behaviour, what private protects for can be seen in the example below:

public class CarProperties {

    public String make;
    private String year;

    public CarProperties(String make, String year)  {
        this.make = make;
        this.year = year;
    }

}

CarProperties properties = new CarProperties("Volvo", "1997");
properties.make = "Toyota"; // this will work, it is a public variable and not private
properties.year = "2000"; // this will not work, the variable year is private and cannot be accessed from outside the class without accessor methods.

A good resource to understand this more thoroughly: https://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html

Private member in a thread getting changed from another thread

Main.java:

Manager.java:

Worker.java:

Output:

2 Answers2