3

I am creating a task poller which looks for tasks every minute. It looks like this:

public class Poller {
    private final ExecutorService e = Executors.newSingleThreadExecutor();

    public void start() {
        e.submit(() -> {
            while (!Thread.currentThread().isInterrupted()) {
                final Task task = manager.getTask();

                if (task != null) {
                    // ...
                } else {
                    try {
                        TimeUnit.MINUTES.sleep(1);
                    } catch (InterruptedException e) {
                        Thread.currentThread().interrupt();
                    }
                }
            }
        });
    }

    public void stop() {
        e.shutdownNow();
    }
}

When I want to stop the poller process, I exucute stop(). This triggers an interrupt on the running thread, which will stop the poller.

This works fine. However, I am wondering if there is a possibility that this thread will be interrupted by someone else, most likely the JVM. In that case, the poller will quit while it shouldn't.

I know we can have spurious wakeups, so we have to guard a wait() with a while (condition) { ... } instead of if (condition) { ... }. Can it also occur that the JVM will interrupt an ongoing thread for no good reason?

In that case, I should introduce a volatile boolean and check on that instead of while (!Thread.currentThread().isInterrupted())?

Franz They
  • 945
  • 9
  • 17
  • 1
    Why don't you use a `ScheduledFuture future = ScheduledExecutorService.schedule()` and eventually call `future.cancel()` in case you wanted to stop the polling to avoid low-level thread handling? Or it's just curiousity? – Ewald Benes Nov 08 '17 at 10:19
  • @EwaldB. There is a difference: `executor.shutdownNow()` will do `future.cancel(true)` on every running future inside, afaik. However, only calling `future.cancel(true)` is not enough, the executor thread must also be stopped in order to have a graceful shutdown, hence a `executor.shutdownNow()` is required. – Franz They Nov 08 '17 at 10:23
  • The difference is that you don't need to implement the busy-loop waiting (calling `Thread.sleep` what I discourage) and there's no need to check any Thread interrupted flags at all. Just do your logic inside the Runnable/Callable block and that's it. Or do you want to get rid of when the VM process quits and gracefully release resources inside your app? – Ewald Benes Nov 08 '17 at 10:30
  • So you mean that I could schedule a task every minute to a `ScheduledExecutorService` instead of submitting one single task which sleeps for one minute and then repeats its execution? – Franz They Nov 08 '17 at 10:33
  • 1
    Yeah! Just call `ScheduledExecutorService.scheduleAtFixedRate(Runnable, 0, 1, TimeUnit.MINUTES)`. If you want to be pragmatic and not learn for academic reasons then this should be the way to go. See the [Javadoc](https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ScheduledExecutorService.html) for more details. – Ewald Benes Nov 08 '17 at 10:36
  • Yeah, that makes sense. I was mostly interested in the concept of spurious interrupts, so I did not really design this *right*. – Franz They Nov 08 '17 at 11:13

1 Answers1

5

Yes, you should be introduce a volatile boolean to control the loop. Not just because of the possibility of a spurious interrupt, but because it makes the code more readable.

You're not achieving a clever optimization by "saving" a boolean and using the interrupt status of the thread to control the loop, you're making it less obvious as to what the working mechanism of the class is.

Edit: I wasn't really answering the core question (mixed it up with wakeups). As we know, spurious wakeups can and do happen, which is why we use guard clauses/loops to check that a condition has really been satisfied.

Spurious interrupts aren't a thing, meaning there's always one thread interrupting another inside the Java ecosystem (unlike with wakeups, where the reason is external). Interrupting random threads is not something that the Java platform does by itself, but you could write a thread that just randomly selects other threads and tries to interrupt them. However that wouldn't be a spurious interrupt, that would be a malicious interrupt.

Kayaman
  • 72,141
  • 5
  • 83
  • 121
  • 3
    I understand that and I agree with that, but the main question mostly is: `spurious interrupts` can occur? – Franz They Nov 08 '17 at 09:41
  • 2
    Well, spurious wakeups you're familiar with, and as you know the reason for those is completely external to the Java platform. I've never heard of "spurious interrupts", and there's no indication anywhere that an `InterruptedException` could result from anything but another thread interrupting this thread. I can't really see threads interrupting other threads randomly in a normal system. Maybe in special circumstances like when running under a debugger, but even then that's just a guess. I'll see if I can find any more information. – Kayaman Nov 08 '17 at 09:56
  • Makes sense... I did not find anything related so far. – Franz They Nov 08 '17 at 10:19
  • 1
    How does checking the interrupt status obscure the intent? That's literally the point of thread interruption. – shmosel Nov 09 '17 at 05:40
  • @shmosel Because the intent is to *stop* the poller, not to interrupt a thread. The interruption is a side-effect, an implementation detail if you will. – Kayaman Nov 09 '17 at 05:49
  • The thread *is* a poller. Stopping the polling ends the thread. I don't understand the distinction you're making. – shmosel Nov 09 '17 at 05:50
  • @shmosel Imagine you get the code in front of your eyes. Which is quicker to grasp `while(!stopped)` or `while(!Thread.currentThread().isInterrupted())`? Imagine you're a junior developer, who isn't quite confident with threads, interruptions (and when the interrupt flag is cleared vs. isn't) etc. Throwing in an extra boolean has no downside, but it makes the code a lot more descriptive. – Kayaman Nov 09 '17 at 05:55
  • 1
    You still need interruption to break out of `sleep()`. I don't see how throwing another flag into the mix is supposed to make things easier to follow. – shmosel Nov 09 '17 at 06:01
  • @shmosel no, the interruption occurs no matter what we do. It's the choice between wanting to use a distinct variable or piggyback logic on the interrupted flag. I would avoid piggybacking, because it's 2017 and we can afford to write descriptive code. This is only about what looks clearer, nothing to do with interrupts or the logic. The logic and interrupts remain the same (well, besides setting our own flag instead of the thread interrupt flag). – Kayaman Nov 09 '17 at 06:04