Thread not stop on calling stop() method

Question

I have created sample program of java Thread in which i am using stop() method to stop the thread using below program

public class App extends Thread
{
    Thread th;

    App(String threadName)
    {
        th = new Thread(threadName);
    }

    public synchronized void run() // Remove synchronized
    {
        for (int i = 0; i < 5; i++) {
            System.out.println(th.getName()+" "+i);
            try {
                Thread.sleep(500);
            } catch (InterruptedException e) {
            }
        }
    }

    public static void main( String[] args ) 
    {
       App thread_1 = new App("Thread-1");
       thread_1.start();
       thread_1.setPriority(MAX_PRIORITY); //Comment this
       thread_1.stop();
       App thread_2 = new App("Thread-2");
       thread_2.start();
    }
}

The output of the above program is :

Thread-1 0
Thread-1 1
Thread-1 2
Thread-1 3
Thread-1 4
Thread-2 0
Thread-2 1
Thread-2 2
Thread-2 3
Thread-2 4

i.e. thread_1 is not stopped. When i am removing synchronized or priority in the code thread is stopped immediately and output will be

Thread-2 0
Thread-2 1
Thread-2 2
Thread-2 3
Thread-2 4

I am not able to understand why it is working like this.

Answer 1

Most of the public methods of the Thread class are synchronized on the Thread instance itself. http://hg.openjdk.java.net/jdk6/jdk6/jdk/file/5672a2be515a/src/share/classes/java/lang/Thread.java

Your run() method is synchronized on the Thread instance. The stop() method calls stop(Throwable), which is also synchronized on the Thread instance, its signature is:

@Deprecated
public final synchronized void stop(Throwable obj) {

The synchronization prevents the main thread from entering thread_1.stop() while the thread itself is still running in your synchronized run() method.

---

This is an example of why it's wise to always use private objects for synchronization. E.g., do this...

class Foobar {
    private final Object lock = new Object();

    public void do_something() {
        synchronized(lock) {
            ...
        }
    }
}

Instead of doing this...

class Foobar {
    public synchronized void do_something() {
        ...
    }
}

The second version is more verbose (Welcome to Java!), but it prevents the user of your Foobar class from using it as a synchronization object in a way that interferes with its own use of itself as a synchronization object.

Answer 2

Thread.stop() is deprecated. consider using this instead:

public class App extends Thread
{
    Thread th;
    volatile boolean bStopThread;
    App(String threadName)
    {
        th = new Thread(threadName);
        bStopThread = false;
    }

    public void stopThread(){
        bStopThread = true;
    }

    public synchronized void run() // Remove synchronized
    {
        for (int i = 0; i < 5; i++) {
            if(bStopThread) return;
            System.out.println(th.getName()+" "+i);
            try {
                Thread.sleep(500);
            } catch (InterruptedException e) {
            }
        }
    }

    public static void main( String[] args ) throws InterruptedException
    {
       App thread_1 = new App("Thread-1");
       thread_1.start();
       thread_1.setPriority(MAX_PRIORITY); //Comment this
       thread_1.stopThread();
       App thread_2 = new App("Thread-2");
       thread_2.start();
    }
}

It should works as you want, although I haven't tested.

Answer 3

You have 3 threads in your application: main thread, running the code of the main method, thread_1 and thread_2. Your main thread starts thread_1 at some point in time X, then calls thread_1.stop() in some point in time Y, Y>X.

Now, what can happen between points X and Y, is that CPU scheduler can decide: "I will now let thread_1 run". Thread_1 will get CPU, will run and will print his text. OR, CPU scheduler can decide: "main thread is now running... let it run". And thread_1 will not get CPU until stop is called and will print nothing.

So you have uncertainty outside of your control about CPU scheduling. You can only assume that raising the priority of the thread hints scheduler to pick the first of the aforementioned choices.

But. stop is deprecated, so never use that. And don't try to guess the order of the execution of the multiple threads.

Answer 4

Put a try catch in your main method. Print stack trace and message of caught exception. Same in run method. Then java will tell you issue.

MHC's method is better but like I said - sometimes (rarely) when you have no control over the thread, can only call stop. But in this case you do have control over it so MHC method will work nicely.

But I do not see what is the issue with your code - it runs fine in my laptop, maybe you did not clean and re compile? Chane some message so you know latest code is running

I used :

package academic.threads;

public class StopThHighPri extends Thread {
    Thread th;
    volatile boolean bStopThread;

    StopThHighPri(String threadName) {
        th = new Thread(threadName);
        bStopThread = false;
    }

    public void stopThread(Thread t) {
        //bStopThread = true;
        try {
            t.stop();
        } catch (Throwable e) {
            System.err.println(" Stop th " + e + " " + e.getMessage());
        }
    }

    public synchronized void run() // Remove synchronized
    {
        try {
            for (int i = 0; i < 5; i++) {
                if (bStopThread)
                    return;
                System.out.println(th.getName() + " " + i);
                try {
                    Thread.sleep(500);
                } catch (InterruptedException e) {
                }
            }
        } catch (Exception e) {
            e.printStackTrace();
            System.out.println("run err " + e);
        }
    }

    public static void main(String[] args) {
        try {
            System.err.println("Code version 002");
            StopThHighPri thread_1 = new StopThHighPri("Thread-1");
            thread_1.start();
            thread_1.setPriority(MAX_PRIORITY); // Comment this
            thread_1.stopThread(thread_1);
            StopThHighPri thread_2 = new StopThHighPri("Thread-2");
            thread_2.start();
        } catch (Exception e) {
            e.printStackTrace();
            System.out.println("MNain err " + e);
        }
    }
}

Put something like System.err.println("Code version 002");

and change the 002 , 003. so you know latest code is working every time you edit the class. Again for learning this is okay but do not need to use stop here