better understanding of Thread construction race conditions

Question

I went through the Oracle tutorial on concurrency and threading and came across the following lines:

Warning: When constructing an object that will be shared between threads, be very careful that a reference to the object does not "leak" prematurely. For example, suppose you want to maintain a List called instances containing every instance of class. You might be tempted to add the following line to your constructor: instances.add(this); But then other threads can use instances to access the object before construction of the object is complete.

Could any one can explain what these lines mean exactly?

[link](http://docs.oracle.com/javase/tutorial/essential/concurrency/syncmeth.html) — Miko, Mar 14 '12 at 18:19

score 4 · Answer 1 · edited May 23 '17 at 12:21

This is about construction race conditions in multiple thread environments. Another thread can start to use a instance of a class before the constructor has completed or before the various field values have been synchronized meaning that the default values will be seen by other threads even after they seem to have been initialized in the constructor. This is the same problem with the double-check locking and other sharing issues.

Here's some good reading:

Consider the following constructor (partially copied from the IBM article):

public class EventListener {
    private int field;
    public EventListener(EventSource eventSource) {
        // do our initialization
        field = 10;
        // register ourselves with the event source
        eventSource.registerListener(this);
    }
    public onEvent(Event e) { 
        // what is printed here?
        System.out.println("Field = " + field);
    }
}

Because of Java optimization and instruction reordering, there is no guarantee that the field = 10 line completes before the registerListener(this) does. Since most likely the events are being handled by another thread then you cannot determine if the onEvent println will output field as 0 (the default value) or 10.

And even if we are sure that the assignment happened before the registerListener call, because the events are handled in another thread, and there is no synchronization of EventListener then the value may still be 0 in that thread even if it is 10 in the thread that called the constructor.

Only if field was defined as final would the order be guaranteed. Otherwise you will need to somehow synchronize EventListener or use the volatile keyword or AtomicInteger.

For even more reading, search for "safe publication". Letting an object become visible in an inconsistent state is unsafe publication, so there are techniques for safe publication. — Tom Anderson, Mar 14 '12 at 18:13

score 4 · Accepted Answer · answered Mar 14 '12 at 18:37

imagine you have two threads. left thread and right thread. left thread is responsible for creating worker objects, and right thread is responsible for putting them to work.

when the left thread has finished creating an object, it puts the object in a location where right thread can find it. it's a variable, let's call it w (for worker), and for simplicity, let's say it's somehow globally accessible.

the right thread is a loop. it checks if w is not null. if w in fact has a none null value, then the method do is called on it.

the class worker looks like this:

public class Worker {
    private int strength;
    private float speed;
    private String name;
    private String specialty;

    public Worker(int str, float spd, String n, String spc) {
        strength = str;
        speed = spd;
        name = n;
        specialty = spc;
    }

    public void do() {
        System.out.println("Worker " + name + " performs " + strength + " " + specialty + " at " + speed + " times per minute.");
    }
}

so it goes something like this (i've tried illustrating the two threads by setting their respective commands in one column each. i hope it's understandable. remember, only one thread is active at once, so that's why there's always only instructions in one column at a time)

left thread                                         | right thread
----------------------------------------------------|-----------------------------------------------------
                                                    |
Worker newWorker = new Worker(                      |               ...right thread sleeps...
                    4,                              |                           .
                    5.2f,                           |                           .
                    "mmayilarun",                   |                           .
                    "multithreaded programming"     |                           .
                );                                  |                           .
-- control jumps to constructor of Worker --        |                           .
strength = str                                      |                           .
speed = spd;                                        |       !!right thread wakes up and takes the focus!!
                                                    |   
                                                    |   if(w == null) {
                                                    |       Thread.sleep(500);
                                                    |   } else {
                                                    |       //this doesn't happen, since w is still null
                                                    |   }
                                                    |   
                                                    |           ...right thread goes back to sleep...
name = n;                                           |                           .
specialty = spc;                                    |                           .
-- control exits constructor of Worker --           |                           .
w = newWorker;                                      |       !!right thread wakes up and takes the focus!!
                                                    |       
                                                    |   if(w == null) {
                                                    |       //this doesn't happen, since w is NOT null anymore
                                                    |   } else {
                                                    |       w.do();
                                                    |   }
                                                    |

in this scenario, all is well. left thread set w after the constructor of worker had completed. but isn't it stupid to do it like that? imagine the savings we could do if we put that call w = instanceOfWorker inside the worker constructor. then we wouldn't have to worry about remembering to actually set w.

new constructor of worker looks like this:

public Worker(int str, float spd, String n, String spc) {
    w = this;
    strength = str;
    speed = spd;
    name = n;
    specialty = spc;
}

now, the flow of the code could end up looking like this:

left thread                                         | right thread
----------------------------------------------------|-----------------------------------------------------
                                                    |
Worker newWorker = new Worker(                      |               ...right thread sleeps...
                    4,                              |                           .
                    5.2f,                           |                           .
                    "mmayilarun",                   |                           .
                    "multithreaded programming"     |                           .
                );                                  |                           .
-- control jumps to constructor of Worker --        |                           .
w = this;   // danger!!                             |                           .
strength = str;                                     |                           .
speed = spd;                                        |       !!right thread wakes up and takes the focus!!
                                                    |   
                                                    |   if(w == null) {
                                                    |       //this doesn't happen, since w is NOT null at this point
                                                    |   } else {
                                                    |       w.do(); //here, w is not yet a fully initialized object,
                                                    |       //and the output is not the expected (if it works at all)
                                                    |   }
                                                    |   
                                                    |           ...right thread goes back to sleep...
name = n;                                           |                           .
specialty = spc;                                    |                           .
-- control exits constructor of Worker --           |

oracle has a more intricate example with a collection of objects called "instances". that's the only difference.

Can I change the constructor into `public Worker(int str, float spd, String n, String spc) { strength = str; speed = spd; name = n; specialty = spc; w=this; }` ? is it still dangerous now? — Hải Phong, May 24 '13 at 06:29

score 1 · Answer 3 · answered Mar 14 '12 at 18:01

1

This is concurrency problem, if there are 2 or more thread active and invoking same code and using same variables, it might be possible that your constructor or shared code gets accessed by 2 thread simultaneously. Problem is 1 thread expects something and because of another thread gets different result.

answered Mar 14 '12 at 18:01

Rahul Borkar

2,742
3
24
38

In same context, if 1 thread is creating object and another thread is going to use that object, it is possible that first thread goes to sleep due to some reason and second thread executes which is actually using object created by first thread, will create a problem. – Rahul Borkar Mar 14 '12 at 18:03

score 0 · Answer 4 · answered Mar 14 '12 at 18:03

It means you should not "leak" the reference to an object before the constructor has completed. For example, see the following code:

// Constructor
public SomeObject() {
  // A holder class that's external to this class.
  ObjectInstances.LIST.add(this);

  // Further code constructing the object, setting fields, etc.
}

In this example, the reference to the newly-created instance of SomeObject is made available to other code by adding it to an external List. This enables other threads to possibly access the object before the constructor has completed, thus an "incomplete" object may be accessed.

score 0 · Answer 5 · answered Mar 14 '12 at 18:04

This means that if you have a constructor:

class Bob {
  List<Bob> instances = ...

  public Bob(){
    instances.add(this); //is bad 
  }
  public void do Operation(){ ... }
}

This is problematic because if you have multiple threads (say you have 1 Bob object/thread), if in Bob instance 1 you are in the ctor doing instances.add(this) Then in Bob instance 2, you will see Bob in the instances variable. If in Bob 2 you try to do instance.get(i).doOperation(); it will fail because instance 1 of Bob is not fully instantiated yet it is already in the instances variable.

better understanding of Thread construction race conditions

5 Answers5