3

I understand that in general it is a bad idea to start a new thread in a constructor because it could let this escape before it is fully constructed. For example:

public final class Test {

    private final int value;

    public Test(int value) throws InterruptedException {
        start();
        this.value = value;
    }

    private void start() throws InterruptedException {
        for (int i = 0; i < 10; i++) {
            new Thread(new Runnable() {

                @Override
                public void run() {
                    System.out.println("Construction OK = " + Boolean.toString(Test.this.value == 5));
                }
            }).start();
        }
    }

    public static void main(String[] args) throws InterruptedException {
        Test test = new Test(5);
    }
}

This prints (obviously not the same every run):

Construction OK = false
Construction OK = false
Construction OK = false
Construction OK = false
Construction OK = false
Construction OK = false
Construction OK = false
Construction OK = true
Construction OK = true
Construction OK = true

Now IF the start method is the last statement of the constructor AND reordering is prevented by using a synchronized block around the final value initialisation, is there still a risk associated with starting threads from the constructor?

public Test(int value) throws InterruptedException {
    synchronized (new Object()) { // to prevent reordering + no deadlock risk
        this.value = value;
    }
    start();
}

EDIT
I don't think this has been asked before in the sense that the question is more specific than just "Can I start threads in a constructor": the threads are started in the last statement of the constructor which means the object construction is finished (as far as I understand it).

Community
  • 1
  • 1
assylias
  • 321,522
  • 82
  • 660
  • 783

4 Answers4

4

Yes there is, because Test could be subclassed and then start() will be executed before the instance is created. The subclasses constructor may have something more to do.

So the class should be final at least.

Andreas Dolk
  • 113,398
  • 19
  • 180
  • 268
1

In this particular case I would consider marking value as volatile (or use AtomicBoolean) and start the threads after the value is set:

this.value = value;   // this.value.set(value)  if using AtomicBoolean
start();

If going for this slightly dodgy solution, I would make the class final as well, to avoid the problem described by Andreas_D.


Regarding your edit:

[...] which means the object construction is finished (as far as I understand it).

That's right, but consider the following scenario:

Your test-threads are slightly more complex, and accesses a list testList of tests. Now if you do

testList.add(new Test());

the thread started in the constructor may not find the associated test in the list, because it has not yet been added. This is avoided by instead doing

Test t = new Test();
testList.add(t);
t.start();

Related question:

Community
  • 1
  • 1
aioobe
  • 413,195
  • 112
  • 811
  • 826
0

The synchronized(new Object()) does NOT prevent reordering - because the monitor is a local variable, the compiler is actually free to ignore the synchronized block.

In particular, the compiler can prove that it is impossible that two threds would lock on the same monitor (by definition of local variables) and that the synchronized block is therefore redundant and can be ignored.

assylias
  • 321,522
  • 82
  • 660
  • 783
0

In the constructor you call the start method by

start()

of the class. Now you can notice that the method you are calling is of an object of this class which has not been create yet. So you are still passing a reference of an unconstructed object to a method. You have included the methodcall itself in the creation of the object, while any method of an object should be called after the object is constructed completely.

So still there is risk associated with it. Also it was really a very good question.

me_digvijay
  • 5,374
  • 9
  • 46
  • 83
  • I think the question is what would that risk be? The full state of my object has been initialised (in this case only one variable), so what could go wrong? – assylias Mar 29 '12 at 11:56
  • There may some statement is your method which can cause some modification in your object. – me_digvijay Mar 29 '12 at 12:05