2

I would like to know if this piece of code is correct or not. Will this not lead to issues as I am submitting the runnable object to the executor service while constructing the object itself?

public class A implements Runnable {
       public A() {
       Executors.newSingleThreadExecutor().execute(this);
       // some other initializations
   }
}

Will this lead to any issues as we are trying to submit the object to the executor even before creating it completely? If the run() method is called even before all the initializing is done (if at all it's possible), will the variables still be null which were not yet initialized?

Please do not ask me to come up with the complete code, as I have been asking this as a general question which requires clarification.

Ravi
  • 879
  • 2
  • 9
  • 23
  • It's hard to tell whether any minimal piece of code is "correct" or not, but you typically want to decouple the executing/client logic from the actual execution. What happens here when you construct your `Runnable` is that it will immediately run the `run` method in another thread. – Mena Jan 22 '18 at 15:34

3 Answers3

4

Yes, there may be issues. The Executor might read a field that you set in the constructor, even before the corresponding code in the constructor was executed. In general you should not expose this from inside a constructor. Java provides useful guarantees for objects after their constructor finished, but in order to benefit from those you have to wait for the result of new X(...) before using it.

C-Otto
  • 5,615
  • 3
  • 29
  • 62
4

Will this lead to any issues as we are trying to submit the object to the executor even before creating it completely?

For one thing, you can get final variable that are still changing value - that is quite bad per the semantics of final. It can lead to very hard-to-trace concurrency bugs in multi-threaded code.

This code will usually print a few zeros and even the occasional 4, even though the final field a is only ever assigned the value 4 and should never been seen having any other value than 4.

public class A implements Runnable {
    private static ExecutorService threads = Executors.newSingleThreadExecutor();
    final int a;

    public A() {
        threads.execute(this);
        Thread.yield();
        a = 4;
    }

    @Override
    public void run() {
        if (a != 4) {
            System.out.println(a);
        }
    }

    public static void main(String[] args) {
        for (int i = 0; i < 50_000; i++) {
            new A();
        }
        threads.shutdown();
    }
}

If the run() method is called even before all the initializing is done (if at all it's possible), will the variables still be null which were not yet initialized?

Yes, the ones not yet initialized will be null for reference variables, or the default value (0, false, '\0', 0d, 0f, etc.) for the primitive types. It is even possible according to the specifications with long and double fields to see only 32 of the 64 bits initialized (although on 64 bit architectures it is unlikely that you will ever observe this)

Erwin Bolwidt
  • 30,799
  • 15
  • 56
  • 79
  • Is it safe if threads.execute(this) is the last line in the constructor? Or would it be still problematic? – Ravi Jan 23 '18 at 04:28
2

There will almost certainly be issues. What you have is called a "this escape" where you pass a this reference in a ctor to an external method. It's super bad, the object is incompletely constructed at that point and anything could happen.

What you should probably do instead is make the constructor private and use a factory method to get a new instance and execute it, if that's the goal.

public class A implements Runnable {

       public A getNew() {
            A a = new A();
            Executors.newSingleThreadExecutor().execute(a);
            return a;
       }

       private A() {
            // some other initializations
   }
}
markspace
  • 10,621
  • 3
  • 25
  • 39