Thread interrupt not working (Java Android)

Question

Edit: see here!

I have a thread with the Runnable shown below. There is a problem with it which I can't figure out: half of the times I call interrupt() on the thread (to stop it) it does not actually terminate (the InterruptedException is not catched).

private class DataRunnable implements Runnable {
    @Override
    public void run() {
        Log.d(TAG, "DataRunnable started");
        while (true) {
            try {
                final String currentTemperature = HeatingSystem.get("currentTemperature");
                mView.post(() -> showData(currentTemperature));
            } catch (ConnectException e) {
                mView.post(() -> showConnectionMessage());
                break;
            }
            try {
                Thread.sleep(10);
            } catch (InterruptedException e) {
                break;
            }
        }
        Log.d(TAG, "DataRunnable terminated");
    }
}

The problem is the HeatingSystem.get(String) method which does the long network operation. I guess somewhere in that method the interrupted flag is reset but I can't find what statement would do that (I didn't find it mentioned in the references for all classes involved in the method, like HttpURLConnection). The method is below (it is not written by me).

/**
 * Retrieves all data except for weekProgram
 * @param attribute_name
 *            = { "day", "time", "currentTemperature", "dayTemperature",
 *            "nightTemperature", "weekProgramState" }; Note that
 *            "weekProgram" has not been included, because it has a more
 *            complex value than a single value. Therefore the funciton
 *            getWeekProgram() is implemented which return a WeekProgram
 *            object that can be easily altered.
 */
public static String get(String attribute_name) throws ConnectException,
        IllegalArgumentException {
    // If XML File does not contain the specified attribute, than
    // throw NotFound or NotFoundArgumentException
    // You can retrieve every attribute with a single value. But for the
    // WeekProgram you need to call getWeekProgram().
    String link = "";
    boolean match = false;
    String[] valid_names = {"day", "time", "currentTemperature",
            "dayTemperature", "nightTemperature", "weekProgramState"};
    String[] tag_names = {"current_day", "time", "current_temperature",
            "day_temperature", "night_temperature", "week_program_state"};
    int i;
    for (i = 0; i < valid_names.length; i++) {
        if (attribute_name.equalsIgnoreCase(valid_names[i])) {
            match = true;
            link = HeatingSystem.BASE_ADDRESS + "/" + valid_names[i];
            break;
        }
    }

    if (match) {
        InputStream in = null;
        try {
            HttpURLConnection connect = getHttpConnection(link, "GET");
            in = connect.getInputStream();

            /**
             * For Debugging Note that when the input stream is already used
             * with this BufferedReader, then after that the XmlPullParser
             * can no longer use it. This will cause an error/exception.
             *
             * BufferedReader inn = new BufferedReader(new
             * InputStreamReader(in)); String testLine = ""; while((testLine
             * = inn.readLine()) != null) { System.out.println("Line: " +
             * testLine); }
             */
            // Set up an XML parser.
            XmlPullParser parser = Xml.newPullParser();
            parser.setFeature(XmlPullParser.FEATURE_PROCESS_NAMESPACES,
                    false);
            parser.setInput(in, "UTF-8"); // Enter the stream.
            parser.nextTag();
            parser.require(XmlPullParser.START_TAG, null, tag_names[i]);

            int eventType = parser.getEventType();

            // Find the single value.
            String value = "";
            while (eventType != XmlPullParser.END_DOCUMENT) {
                if (eventType == XmlPullParser.TEXT) {
                    value = parser.getText();
                    break;
                }
                eventType = parser.next();
            }

            return value;
        } catch (MalformedURLException e) {
            e.printStackTrace();
        } catch (FileNotFoundException e) {
            System.out.println("FileNotFound Exception! " + e.getMessage());
            // e.printStackTrace();
        } catch (XmlPullParserException e) {
            e.printStackTrace();
        } catch (IOException e) {
            e.printStackTrace();
        } finally {
            if (in != null)
                try {
                    in.close();
                } catch (IOException e) {
                    e.printStackTrace();
                }
        }
    } else {
        // return null;
        throw new IllegalArgumentException("Invalid Input Argument: \""
                + attribute_name + "\".");
    }
    return null;
}

/**
 * Method for GET and PUT requests
 * @param link
 * @param type
 * @return
 * @throws IOException
 * @throws MalformedURLException
 * @throws UnknownHostException
 * @throws FileNotFoundException
 */
private static HttpURLConnection getHttpConnection(String link, String type)
        throws IOException, MalformedURLException, UnknownHostException,
        FileNotFoundException {
    URL url = new URL(link);
    HttpURLConnection connect = (HttpURLConnection) url.openConnection();
    connect.setReadTimeout(HeatingSystem.TIME_OUT);
    connect.setConnectTimeout(HeatingSystem.TIME_OUT);
    connect.setRequestProperty("Content-Type", "application/xml");
    connect.setRequestMethod(type);
    if (type.equalsIgnoreCase("GET")) {
        connect.setDoInput(true);
        connect.setDoOutput(false);
    } else if (type.equalsIgnoreCase("PUT")) {
        connect.setDoInput(false);
        connect.setDoOutput(true);
    }
    connect.connect();
    return connect;
}

Does someone know what in the method above could cause the problem?

The Thread.sleep() also throws InterruptException when interrupt() has been called before entering Thread.sleep(): Calling Thread.sleep() with *interrupted status* set? .

I checked if Thread.sleep() is reached after an interrupt, and it is reached.

This is how DataRunnable is started and interrupted (I do always get the "onPause called" log):

@Override
public void onResume() {
    connect();
    super.onResume();
}

@Override
public void onPause() {
    Log.d(TAG, "onPause called");
    mDataThread.interrupt();
    super.onPause();
}

private void connect() {
    if (mDataThread != null && mDataThread.isAlive()) {
        Log.e(TAG, "mDataThread is alive while it shouldn't!"); // TODO: remove this for production.
    }
    setVisibleView(mLoading);
    mDataThread = new Thread(new DataRunnable());
    mDataThread.start();
}

Answer 1

I have been struggling with similar problems in the past, and I never found a satisfying solution to this. The only workaround that always worked for me was introducing a volatile boolean "stopRequested" member variable that is set to true in order to interrupt the Runnable and checked in the while condition of the Runnable instead of the Thread's interrupted status.

The unfortunate detail about my experiences with this is that I have never been able to extract a small compilable example that reproduces this problem. Are you able to do that? In case you are not able to do that, I'd be interested to know if you are positive that you are working with the right instance of mDataThread? You could verify this by logging its hashcode...

Answer 2

You'll need to change your while loop to:

// You'll need to change your while loop check to this for it to reliably stop when interrupting.
while(!Thread.currentThread().isInterrupted()) {
    try {
        final String currentTemperature = HeatingSystem.get("currentTemperature");
        mView.post(() -> showData(currentTemperature));
    } catch (ConnectException e) {
        mView.post(() -> showConnectionMessage());
        break;
    }

    try {
        Thread.sleep(10);
    } catch (InterruptedException e) {
        break;
    }
}

Instead of while(true).

Note that Thread.interrupted() and Thread.currentThread().isInterrupted() do different things. The first one resets the interrupted status after the check. The latter leaves the status unchanged.

Answer 3

This is not really an answer but I don't know where to put it. With further debugging I think I encountered odd behavior and I was able to reproduce it in a test class. Now I'm curious whether this actually is wrong behavior as I suspect, and whether other people could reproduce it. I hope that it works to write this as an answer.

(Could it be better to make this into a new question, or actually just file a bug report?)

Below is the test class, it has to be ran on Android since it's about the getInputStream() call which behaves differently on Android (don't know exactly why). On Android, getInputStream() will throw an InterruptedIOException when interrupted. The thread below loops and gets interrupted after a second. Thus when it gets interrupted, the exception should be thrown by getInputStream() and should be catched using the catch block. This works correctly sometimes, but most of the times the exception is not thrown! Instead only the interrupted flag is reset, thus changed from interrupted==true to interrupted==false and that is then caught by the if. The message in the if pops up for me. This seems to me to be wrong behavior.

import java.net.HttpURLConnection;
import java.net.URL;

class InterruptTest {

InterruptTest() {
    Thread thread = new Thread(new ConnectionRunnable());
    thread.start();
    try {
        Thread.sleep(1000);
    } catch (InterruptedException e) {}
    thread.interrupt();
}

private class ConnectionRunnable implements Runnable {
    @Override
    public void run() {
        while (true) {
            try {
                URL url = new URL("http://www.google.com");
                HttpURLConnection connect = (HttpURLConnection) url.openConnection();

                boolean wasInterruptedBefore = Thread.currentThread().isInterrupted();
                connect.getInputStream(); // This call seems to behave odd(ly?)
                boolean wasInterruptedAfter = Thread.currentThread().isInterrupted();

                if (wasInterruptedBefore == true && wasInterruptedAfter == false) {
                    System.out.println("Wut! Interrupted changed from true to false while no InterruptedIOException or InterruptedException was thrown");
                    break;
                }
            } catch (Exception e) {
                System.out.println(e.getClass().getName() + ": " + e.getMessage());
                break;
            }
            for (int i = 0; i < 100000; i += 1) { // Crunching
                System.out.print("");
            }
        }
        System.out.println("ConnectionThread is stopped");
    }
}
}