1

Can anyone explain to me why the first thread doesn't work and the second works perfectly:

public class Test {

    public static void main(String args[]) throws InterruptedException {
        TestThread1 t1 = new TestThread1();
        TestThread2 t2 = new TestThread2();

        t1.startThread();
        t2.start();

        Thread.sleep(4000);

        t1.stopThread();
        t2.stopThread();
    }
}

class TestThread1 extends Thread {

    private volatile TestThread1 thread;

    public void startThread() {
        thread = new TestThread1();
        thread.start();
    }

    public void run() {
        while (thread != null) {
            System.out.println("RUNNING 1 ...");
        }
    }

    public void stopThread() {
        thread = null;
    }
}

class TestThread2 extends Thread {

    private volatile boolean finished = false;

    public void run() {
        while (!finished) {
            System.out.println("RUNNING 2 ...");
        }
    }

    public void stopThread() {
        finished = true;
    }
}

When I debug inside TestThread1 class: Inside startThread, the thread member is filled (so it is not null), inside run, thread member is null!!! And finally, inside stopThread, the thread member is not null!!!

Can anyone explain to me what is happening here?

Chris Mantle
  • 6,595
  • 3
  • 34
  • 48
Naruto Biju Mode
  • 2,011
  • 3
  • 15
  • 28

5 Answers5

5

Here, you have two instances of TestThread1 t1:

  • One is stored into your t1 local variable (in your main method).
  • One is stored into your thread instance variable (of t1).

t1 is never started, t1.thread is.

t1.stopThread() sets t1.thread to null, but it doesn't affect t1.thread.thread.

Since you're starting t1.thread, its run method is using t1.thread.thread:

  • This is never set to anything (so it's using null).
  • Calling t1.stopThread() like you do would only set t1.thread to null, which wouldn't affect t1.thread.thread.

More generally, you can't just "kill" a thread as such, but you can implement tests within the method to tell it to return under certain circumstances. What you've done with your second test is closer to this (using while (!finished) { ... } with a volatile variable).

I wouldn't limit the test to finished. It's also useful to test whether the thread was interrupted, in particular because if you run your runnables within an ExecutorService shutdownNow() will try to interrupt them (see this question).

I'd use while (!finished && !Thread.currentThread().isInterrupted()) { ... }.

(Note the difference between Thread.currentThread().isInterrupted() and Thread.interrupted(): they may seem similar, but the latter will also reset the status, which you might not want.)

Depending on what's within your loop (or whether there is a loop at all), you may want to use something like if (finished || Thread.currentThread().isInterrupted()) { return; } at various strategic points, where it makes sense.

Community
  • 1
  • 1
Bruno
  • 119,590
  • 31
  • 270
  • 376
  • In the fact i wanted to implement ths solution to deprecated stop method which is explained in this page http://docs.oracle.com/javase/7/docs/technotes/guides/concurrency/threadPrimitiveDeprecation.html – Naruto Biju Mode May 21 '14 at 16:39
  • @Braj, what do you mean? – Bruno May 21 '14 at 16:39
  • @Bruno can you look to the link i sent and help me to implement that solution please. – Naruto Biju Mode May 21 '14 at 16:40
  • Have you tried it? It is stopped in both the cases. – Braj May 21 '14 at 16:40
  • 1
    @Braj, sure, `TestThread1` will not even enter its loop at any point: `t1.thread.thread` is always `null`, as I said. – Bruno May 21 '14 at 16:42
  • The first thread can't enter to the loop because thread member is null. – Naruto Biju Mode May 21 '14 at 16:42
  • @Bruno can you help to implement the solution explained in this link http://docs.oracle.com/javase/7/docs/technotes/guides/concurrency/threadPrimitiveDeprecation.html in the section What should I use instead of Thread.stop? – Naruto Biju Mode May 21 '14 at 16:45
  • @Bruno But what i read is that working with interrupt flag is not good practice because that flag can be reset by any method which throw InterruptedException – Naruto Biju Mode May 21 '14 at 16:52
  • Indeed, that's what I'd use both `finished` and check for interruption, and also catch and exit/re-interrupt when handling this exception. – Bruno May 21 '14 at 16:54
  • @Bruno i understand, but my objective for using a thread member is that i want to set that thread member to null inside the stopThread because from what i read it's the best way to kill safely a thread. – Naruto Biju Mode May 21 '14 at 16:58
  • @Bruno The second thread doesn't set the thread member to null (because it doesn't have one), so the thread instance will not be taken by the finalizer to liberate all resources allocated by the thread. – Naruto Biju Mode May 21 '14 at 17:00
  • Why would you want a thread member within your thread (or runnable)? Losing a reference to a thread doesn't make it stop if it's still doing something. Not sure where you got your references from. – Bruno May 21 '14 at 17:04
  • @Bruno According to this link http://docs.oracle.com/javase/7/docs/technotes/guides/concurrency/threadPrimitiveDeprecation.html what i understand : to kill a thread correctly it should be set to null to avoid having damaged object, because having null object will liberate all resources allocated by the thread. – Naruto Biju Mode May 21 '14 at 17:38
  • Just remembered this... You might be interested in [this question](http://codereview.stackexchange.com/q/52454/9735). Getting the "Java Concurrency in Practice" book is always a good idea. – Bruno Jun 05 '14 at 14:24
0
TestThread1 t1 = new TestThread1();
t1.startThread();

This will simply call method startThread() on object t1. Inside this method you are creating a new Thread.

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

But for this Thread thread instance variable is null(It is not null for t1).

So in both cases thread variable should be null.

Aniket Thakur
  • 66,731
  • 38
  • 279
  • 289
0

There is two TestThread1 object being created, one is started and the other is stopped.

I suggest not extending Thread and instead wrapping your Runnable once.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
0

Because in your main method you create a Thread1 Object. You then run startThread which creates a different Thread1 object inside the first one and sets it to the field thread. You then start the second object which didn't have its own thread field initialized. When run method is run on the second object the condition is false and the while loop doesn't start.

Your object hierarchy looks something like this

t1 (Thread1) {
    thread(Thread1): {
        thread: null;
        run() {
             while (thread != null) {...} // this is the method that is run - thread is null here since you never initialized it
        }
   };
   startThread() {} // calls the run method on the nested thread object above
   run() {
        while (thread != null) {...} // this method is not run since t1.start() is never called in main()
   }
}
Andrei Fierbinteanu
  • 7,656
  • 3
  • 31
  • 45
0

in your case,

 t2.start();

is calling run method directly, it is not creating thread inside startThread method.

so t1.stopThread() makes the volatile thread inside Thread1 class null. so you are getting like that.

solution

use

 t1.startThread();
        t2.startThread();

instead of

 t1.startThread();
        t2.start();

which makes 2 threads to create separate threads inside that method.

if you want a single thread of Thread1 then use runnable interface and create 2 threads and call startThread respectively rather than creating extra threads inside main.

Karibasappa G C
  • 2,686
  • 1
  • 18
  • 27