51

I am learning how to use threads in Java. And I wrote a class that implements Runnable to run concurrently to another thread. The main thread handles listening to the serial port where as the second thread will handle sending data to that same port.

public class MyNewThread implements Runnable {
    Thread t;

    MyNewThread() {
        t = new Thread (this, "Data Thread");
        t.start();
    }

    public void run()  {
        // New Thread code here 
    }

There first thread starts the second like this:

public class Main {
    public static void main(String[] args) throws Exception{
        new MyNewThread();
        // First thread code there
    }  
}

This works but my complier flags a warning saying: It is dangerous to start a new thread in the constructor. Why is this?

The second part to this question is: how if I have a loop running in one thread (the serial port listen thread) and I type an exit command in my second thread. How do I get the first thread to terminate? Thanks.

Gray
  • 115,027
  • 24
  • 293
  • 354
Zac
  • 2,229
  • 9
  • 33
  • 41
  • 11
    It would be better to fork the second part of your question into another thread, er, I mean, question. – R. Martinho Fernandes Apr 11 '11 at 15:06
  • 3
    Consider asking an actual second StackOverflow question for the second part of the question. That will get you the best answers for each one. – Emil Sit Apr 11 '11 at 15:06
  • The first question was answered here: http://stackoverflow.com/questions/84285/calling-thread-start-within-its-own-constructor. I won't vote to close as duplicate because of the second part... Now this gets complicated to separate because there are already answers to that :( – R. Martinho Fernandes Apr 11 '11 at 15:13

4 Answers4

51

To your first question: Starting a thread in a constructor passing in this escapes this. That means that you are actually giving out a reference to your object before it is fully constructed. The thread will start before your constructor finishes. This can result in all kinds of weird behaviors.

To your second question: There is no acceptable way to force another thread to stop in Java, so you would use a variable which the thread would check to know whether or not it should stop. The other thread would set it to indicate that the first thread would stop. The variable has to be volatile or all accesses synchronized to ensure proper publication. Here is some code which would be something like what you want.

public class MyNewThread implements Runnable {

    private final Thread t;
    private volatile boolean shouldStop = false;

    MyNewThread() {
        t = new Thread (this, "Data Thread");
    }

    public void start() {
        t.start();
    }

    public void stop() {   
         shouldStop = true;
    }

    public void run()  {
         while(!shouldStop)
         {
             // do stuff
         }
    }
}

Whatever wants to create and start the thread would do:

MyNewThread thread = new MyNewThread();
thread.start();

Whatever wants to stop the thread would do:

thread.stop();
Justin Waugh
  • 3,975
  • 1
  • 21
  • 14
  • 1
    Starting a thread in a constructor only publishes this if you actually publish this (as in the code above, and even then, it's the constructor call that does the publishing). `foo = new SomeSpecialThread(); foo.start();` doesn't publish this, or does it? Now, starting a thread on **its own** constructor does indeed expose an incomplete object. – R. Martinho Fernandes Apr 11 '11 at 15:08
  • I expect you meant to make the field `shouldStop` volatile. – seh Apr 11 '11 at 15:14
  • 1
    @seh: no need for that as all accesses to it are synchronized. @Justin: wouldn't it be better to just inherit from `Thread`? – R. Martinho Fernandes Apr 11 '11 at 15:16
  • @seh Making it volatile is not enough. To use a volatile without synchronization you need to ensure that only a single thread will ever be modifying it, which would not be the case here. – Justin Waugh Apr 11 '11 at 15:18
  • Yes, I see that, but the `synchronized` monitor usage is overkill; no thread wants to wait on that condition changing. And Justin, what makes you think that only one thread could change the field? It only ever moves through one transition: from false to true. Two threads changing the same volatile value from false to true will not interfere with one another. – seh Apr 11 '11 at 15:19
  • @Martinho Fernandes: You certainly can inherit from Thread, but then you've possibly unnecessarily locked yourself into that inheritance path – Justin Waugh Apr 11 '11 at 15:19
  • @seh: it's not about calling wait()/notify(). It is simply about publishing the state of an object. If one thread sets a value on variable, and that variable is not volatile or synchronized, it may never become visible to other threads. – Justin Waugh Apr 11 '11 at 15:21
  • @seh: you might modify it in the future to make it a state enum or something that is not just true/false. Why not make it futureproof from the start? – Justin Waugh Apr 11 '11 at 15:22
  • Justin, I think you're arguing with the wrong person here. Please read what I wrote again: I recommended that field `shouldStop` be made volatile and that its access not be synchronized. Its sole use is to tell the main thread loop when to stop, which a volatile value does best. Again, the synchronization is not necessary here. – seh Apr 11 '11 at 15:24
  • @seh: I agree with what you are saying. I just find myself tending to end up adding stuff like pausing and resuming and so forth to things, and have to end up allowing more than just simple checking for stop, but I'll change the example here for simplicity's sake – Justin Waugh Apr 11 '11 at 15:31
  • 3
    @Justin, "To use a volatile without synchronization you need to ensure that only a single thread will ever be modifying it, which would not be the case here." ... this is wrong. Changes to volatile variables are visible to other threads without further synchronization. Writing a volatile variable and subsequent reading same variable create "happens-before" relationship which is guaranteed to be visible in Java Memory Model. See http://download.oracle.com/javase/6/docs/api/java/util/concurrent/package-summary.html#MemoryVisibility for details (or JMM specs). – Peter Štibraný Apr 11 '11 at 15:33
  • 1
    @Peter You are certainly right, I misread the statement in JCIP. It is only the case when writes to the variable do depend on its current value that you have to ensure only a single thread is updating the value – Justin Waugh Apr 11 '11 at 15:35
  • `.stop()` is deprecated – shareef May 02 '15 at 12:16
  • 1
    @shareef: The `.stop()` that you see here is not the one from `java.lang.Thread`, but an independent one. While the name pair `start` and `stop` is a really nice one, you are right that it should not be used in the context of Java threads, since it looks like the forbidden `Thread.stop()` method. – Roland Illig Sep 11 '16 at 20:10
12

Lets take a look at a basic example:

class MyClass implements Runnable{
   int a = 0;
   String b = null;

   public MyClass(){
       new Thread(this).start();
       b = "Foo";
   }

   public void run(){
      a = b.length(); //can throw NullPointerException
   }
}

In this instance the MyClass.this is said to escape the constructor. That means that the object is available to reference but all of its fields that are being built in the constructor may not be created. To take this to another level what if b was final You would expect it to be available but it is not ensured. This is known as a partially constructed objects and is perfectly legal in java.

John Vint
  • 39,695
  • 7
  • 78
  • 108
0

about second question , you can check if second thread has been terminated or not by isAlive method and if yes use break keyword to turn off the looping for first thread then will be terminated if nothing has to do

public class MyNewThread implements Runnable {
Thread t;

MyNewThread() {
    t = new Thread (this, "Data Thread");
    t.start();
}

public void run()  {

   reading code ................
    // New Thread code here 
}

public class Main {
public static void main(String[] args) throws Exception{
   MyNewThread thread = new MyNewThread();

while(true)
{
    listening code ...................

    if(!thread.t.isAlive())
      break;
 }

}  
}
Kara
  • 6,115
  • 16
  • 50
  • 57
user1841718
  • 187
  • 1
  • 4
  • 15
  • Even with the questionable stability of launching and starting a thread in a class constructor, this suggestion of performing a myClass.isAlive() check is not a bad way to have a main() method wait while the class thread runs. It should be called in a while loop with sleep pauses, contained in a try/catch block, looking for an InterruptedException. – Robert Casey Mar 04 '15 at 22:01
-5

The second part to this question is: how if I have a loop running in one thread (the serial port listen thread) and I type an exit command in my second thread. How do I get the first thread to terminate?

Have it keep looping until a condition has been reached. For example:

public void run() {
    while ( !inputConsole.getCommand().equals("exit") ) {
        //Do Something
        Thread.sleep(1000); //put thread to sleep for 1 second
    }
}
Mike Thomsen
  • 36,828
  • 10
  • 60
  • 83