Threads and interrupt

Question

I have the following code

package threads;

import java.util.ArrayList;
import java.util.List;

public class Threads extends Thread implements Runnable {

    private final List<Thread> threadList = new ArrayList<>();
    private String f, l;
    private Thread greetings1, greetings;

    public static void main(String[] args) {

        String[] elements = { "Tim", "Fred", "Jeff", "Scott" };
        Threads t = new Threads();
        for (String e : elements) {
            t.threadL(e);

            t.threadR(e);
        }

        for (int index = 0; index < t.threadList.size(); index++) {
            // blank at the moment

        }
    }

    public void threadR(String f) {
        greetings = new Thread(f);

        Thread greetingsFromFred = new Thread(greetings) {

            @Override
            public void run() {
                for (int i = 1; i < 5; i++) {
                    try {
                        System.out.println("Main Thread: " + i + " " + f);
                        Thread.sleep(5000);

                    } catch (InterruptedException ex) {
                        System.out.println("Main thread interrupted.");

                    }
                    System.out.println("Main thread exiting.");
                }
            }
        };
        greetingsFromFred.start();
    }

    public List<Thread> threadL(String l) {
        greetings1 = new Thread(this);

        this.l = l;
        greetings1.start();

        threadList.add(greetings1);

        return (threadList);

    }

    @Override
    public void run() {

        greetings.interrupt(); // interrupt greetings thread here <-------

        System.out.println("Greetings from " + l + "! threadL");

    }
}

when threadL runs i want to interrupt threadR from running and therefore cause the

 System.out.println("Main thread interrupted.");

to be printed from threadR

I have highlighted in my code where the interrupt should take place

 greetings.interrupt();  //interrupt greetings thread here <-------

why is the interruption not working as it currently stands?

Answer 1

As noted by others, there are several issues here.

Unnecessary Inheritance

Your class Threads has a run() method that interrupts whatever thread is assigned to greetings, but neither R and L inherit that run() method. R extends Thread and overrides run(), and L just extends Thread without overriding. That structure makes no sense.

Given that, it seems that this run() method should be overridden by L.

If you make that happen, then there is no need for Threads to override Thread or implement Runnable. You can take that off entirely and have R and L each override Thread themselves (using the Thread(String) constructor).

public class Threads {
...
    public List<Thread> threadL(String l) {
        greetings1 = new Thread(l) {
            @Override
            public void run() {
                greetings.interrupt(); // interrupt greetings thread here <-------
                System.out.println("Greetings from " + l + "! threadL");
            }
        ...
    }
}

So now threadR() creates and starts a new Thread that will be interrupted, and threadL() creates and starts a new Thread that will interrupt, and Threads just exists for the main() method and the creation/starting of threads in threadR() and threadL(). Cleaner.

Interrupting the Wrong Thread

As noted by others, you have this unnecessary variable, greetingsFromFred, that just complicates matters. Everything you do for greetingsFromFred you should instead do for greetings (except the constructor).

public void threadR(String f) {
    greetings = new Thread(f) {

        @Override
        public void run() {
            ...
        }
    greetings.start();
}

Race Condition on greetings

Here's the most subtle part, IMO. You're using the static variable greetings to hold the current Thread object R that the current L is supposed to interrupt. But there's no coordination between those two threads. There's no reason to believe that each L is going to see a unique R in greetings. And that's a Very Bad Thing. What you need to do is make greetings volatile, then make R wait until greetings is null before assigning it and make L wait until greetings is not null before interrupting it.

private volatile Thread greetings;
...
public void threadR(String f) {
    while (greetings != null);
    greetings = new Thread(f) {
...
public void threadL(String l) {
    Thread greetings1 = new Thread(l) {
        @Override
        public void run() {
            while (greetings == null);
...

Note the use of while(...);. The trailing semicolon is an empty statement. This construct just says to keep checking the condition until it's false, then continue. Since R never proceeds until it has a null and then sets it, and L never proceeds until it has a non-null and then nulls it, they will always remain in sync.

Answer 2

Your program is a bit of a maze of thread objects. Let's try to follow it from main.

In main, you create an object t which is of type Threads. This means it's a Thread and a Runnable.

Then, for each of your string, you run t's threadL and threadR with that string as parameter.

Nowhere in main do you actually start t, or even run its run method directly.

Then, in threadR you create a new, empty Thread and assign it to greetings.

You then use that new empty Thread (greetings) as a Runnable that's being passed to a new Thread object that also has its run method overridden. This is rather futile. In any case, greetingsFromFred is started, and will run the loop. But greetings still contains that empty Thread that has nothing to do with the greetingsFromFred thread.

In threadL you create yet another Thread, to which you pass the current Thread (which is t in main) as its runnable. You then finally start it. It will try to interrupt the thread in greetings, but as we said, that's an inactive empty thread that was never started.

Your structure should be a lot less convoluted than that. As much as possible, use just Runnable objects for things that should be executed. Thus, Threads itself shoudld be a Runnable, and threadR should probably be fixed to something like this:

public void threadR(String f) {

    Runnable greetingsFromFred = new Runnable() {

        @Override
        public void run() {
            for (int i = 1; i < 5; i++) {
                try {
                    System.out.println("Main Thread: " + i + " " + f);
                    Thread.sleep(5000);

                } catch (InterruptedException ex) {
                    System.out.println("Main thread interrupted.");

                }
                System.out.println("Main thread exiting.");
            }
        }
    };

    greetings = new Thread(greetingsFromFred, f);
    greetings.start();
}

This way, greetings will be a Thread which will run that loop that you have created in a Runnable, and also have the name f.

Note, though: since you don't actually stop the loop in your catch clause, you won't exit the loop when the thread is interrupted, and you'll possibly have the "Exiting" message printed more than once, depending on when the interrupt hits the loop.

Answer 3

The thread you want to interrupt is called greetingsFromFred not greetings so this change will work, provided you make greetingsFromFred visible.

       greetingsFromFred.interrupt();  //interrupt greetings thread here <-------

Answer 4

You'd need to interrupt greetingsFromFred. This is the thread that is being started and actually run. The thread greetings is never run. Is is only passed as runnable to the other thread. Why do you even pass some other thread in a thread's constructor? This whole design is very fishy. I think you got confused about thread/runnable and when and how to use one.