Geek Answers Handbook

Why shouldn't I use Thread.start () in the constructor of my class?

I was looking for justification as to why you shouldn't call the thread start method inside the constructor for the class. Consider the following code:

class SomeClass { public ImportantData data = null; public Thread t = null; public SomeClass(ImportantData d) { t = new MyOperationThread(); // t.start(); // Footnote 1 data = d; t.start(); // Footnote 2 } }

The important information is a common set of files (presumably important), and MyOperationThread is a subclass of the thread that knows how to handle SomeClass instances.

Footnodes:

  • I fully understand why this is unsafe. If MyOperationThread tries to access SomeClass.data before the next statement finishes (and the data is initialized), I will get an exception for which I was otherwise unprepared. Or maybe I won’t do it. You cannot always speak with the help of threads. In any case, I set myself up for strange, unexpected behavior later.

  • I don’t understand why to do this, it is a forbidden territory. At this point, all SomeClass members were initialized, no other member functions that changed state were called, and the construction was thus completed effectively.

From what I understand, the reason he considered it bad practice is that you can "leak a reference to an object that has not yet been completely built." But the object was completely built, the designer has no choice but to return. I searched for other questions, looking for a more specific answer to this question, and also looked at the material that is referenced, but did not find anything that says "you should not, because such and such undesirable behavior", only what they say " you should not".

As if the beginning of the stream in the constructor was conceptually different from this situation:

 class SomeClass { public ImportantData data = null; public SomeClass(ImportantData d) { // OtherClass.someExternalOperation(this); // Not a good idea data = d; OtherClass.someExternalOperation(this); // Usually accepted as OK } }

How else aside, what if the class was final?

 final class SomeClass // like this { ...

I had a lot of questions that asked about this, and answers that you do not need, but no one offered an explanation, so I decided that I would try to add one that has a few more details.

+3
java multithreading constructor thread-safety
source share
2 answers

But the object is completely constructed, the designer has no choice but to return

Yes and no. The problem is that according to the Java memory model, the compiler can change the order of constructor operations and actually complete the constructor of the object after the constructor completes. The volatile or final fields are guaranteed to be initialized before the constructor completes, but there is no guarantee that (for example) your ImportantData data field will be correctly initialized by the time the constructor completes.

However , as pointed out in the @meriton comments, it comes before the relationship with the thread and the thread that started it. In case # 2, you're fine, because data must be assigned completely before the start of the stream. This is guaranteed by the Java memory model.

Nevertheless, it is considered improper practice to “leak” references to an object in its constructor to another thread, because if constructor lines were added after t.start() , this will be a race condition if the stream sees the object is completely built or not.

Here are some more:

+7
source share

Rational, evidence-based argument against this practice

Consider the following situation. You have a class that starts the scheduler thread, which puts tasks in a database encoded similarly to the following:

 class DBEventManager { private Thread t; private Database db; private LinkedBlockingQueue<MyEvent> eventqueue; public DBEventManager() { this("127.0.0.1:31337"); } public DBEventManager(String hostname) { db = new OracleDatabase(hostname); t = new DBJanitor(this); eventqueue = new LinkedBlockingQueue<MyEvent>(); eventqueue.put(new MyEvent("Hello Database!")); t.start(); } // getters for db and eventqueue }

A database is a kind of database abstraction interface, MyEvents are generated by everything that should signal a database change, and DBJanitor is a subclass of Thread that knows how to apply MyEvents to a database. As we can see, this implementation uses the created OracleDatabase class as the database implementation.

This is all good and good, but now your project requirements have changed. Your new plugin should be able to use the existing code base, but should also be able to connect to the Microsoft Access database. You decide to solve this with a subclass:

 class AccessDBEventManager extends DBEventManager() { public AccessDBEventManager(String filename) { super(); db = new MSAccessDatabase(filename); } }

But now, our decision to start the thread in the constructor returns to haunt us now. By launching the client crappy 700MHz single core pentium II, this code now has a race condition: every few starts, a database manager is created, creating databases and starting threads, sending "Hello Database!" event to the wrong database.

This is because the thread starts at the end of the constructor of the superclass ... but this is not the end of the construction, we are still initialized by the constructor of the subclass, which overrides some elements of the superclass, so when the thread jumps directly to the event dispatch in the database, it sometimes gets inside before the subclass constructor updates the database reference to the correct database.

There are at least 2 solutions for this:

  • You can make your class final, which will prevent it from subclassing. If you do this, you can ensure with confidence that your object is completely constructed before exposing it to any other objects (even if it has not left the constructor), thereby ensuring that odd behavior like this will not happen.

    You should also take steps to prevent reordering of assignments in your constructor: you can declare fields that the thread will access as mutable, or you can wrap them in a synchronized block of any type. Each of these two parameters imposes additional restrictions on the fact that reordering can be performed by the JIT compiler, which ensures that the fields are correctly assigned when the thread accesses them.

    In this situation, you are likely to argue with your boss until he allows you to make changes to the code base, which is associated with changing DBEventManager constructors to look something like this:

     private Thread t; // no getter, doesn't need to be volatile private volatile Database db; private volatile LinkedBlockingQueue<MyEvent> eventqueue; public DBEventManager() { this("127.0.0.1:31337"); } public DBEventManager(String hostname) { this(new OracleDatabase(hostname)); } public DBEventManager(Database newdb) { db = newdb; t = new DBJanitor(this); eventqueue = new LinkedBlockingQueue<MyEvent>(); eventqueue.put(new MyEvent("Hello Database!")); t.start(); }

    If you had previously considered this problem in development, you might have added an additional constructor. Then you can make your Microsoft Access using DBEventManager safely using DBEventManager(new MSAccessDatabase("somefile.db"));

  • You can simply not do this and abandon the generally accepted method of using a separate start method and the optional static factory method or methods that call the constructor, and then the start method, for example

  public start() { t.start(); } public static DBEventManager getInstance(String hostname) { DBEventManager dbem = new DBEventManager(hostname); dbem.start(); return DBEventManager; }

I am sure that I am sane, but a second opinion would be nice.

+4
source share

More articles:


All Articles