Java Timer.cancel() not cancelling timer

Question

I have a simple socket server and client set up in Java. I want to implement a method that checks for a connection timeout. Basically, the client sends 01101011 01100101 01100101 01110000 to the server every 5 seconds. The server then checks if it already has a connection established with the client it's received the string from.

If it doesn't, then it adds the client's IP to an array called connections, and starts a 30 second timer. If the 30 second timer reaches the end, it will remove the client from the connections array.

Otherwise, if it does already have a connection established with the client, then it cancels the 30 second timer and restarts it.

This is my code:

//This if statement fires every 5 seconds when it received the string from the client
if(o.equals("01101011 01100101 01100101 01110000")) {
    Timer timeoutTimer = new Timer();

    if(Arrays.asList(connections).contains(connection.getOtherEnd().getAddress())) {
        timeoutTimer.cancel();
        timeoutTimer.purge();
        timeoutTimer = new Timer();
    }
    else {
        connections = ArrayModification.append(connections, connection.getOtherEnd().getAddress());
        System.out.println("Client connected");
    }
    timeoutTimer.schedule(new TimerTask() {
        @Override
        public void run() {
            connections = ArrayModification.remove(connections, connection.getOtherEnd().getAddress());
            System.out.println("Client disconnected");
        }
    }, 30000);
}

It works fine for about 30 seconds, but then, it for some reason executes the timer tasks that should've been cancelled.

Here's my output:

Client connected <- Happens when the client first connects
Client disconnected <- Happens 30 seconds later from the timer task that should've been cancelled
Client connected
Client disconnected
Client connected
Client disconnected
Client connected

I could not figure out why the cancelled timers are still running. Any help would be appreciated!

Answer 1

I figured out how to do it. With the help of commenters, I figured out that I was creating a new Timer every time, and not cancelling the old one. Here is how I fixed it.

I first created an object called Timeout. Here's what it looks like:

import java.util.Timer;
import java.util.TimerTask;

public class Timeout {
    String address;
    Timer timer;
    TimerTask timerTask;

    protected Timeout(String address) {
        this.address = address;
        this.timer = new Timer();
    }

    public void setTimerTask(TimerTask timerTask) {
        this.timerTask = timerTask;
    }

    public String getAddress() {
        return address;
    }

    public void startTimer() {
        timer.cancel();
        timer.purge();
        timer = new Timer();
        timer.schedule(timerTask, 30000);
    }
}

I then changed the code in the server to this:

if(o.equals("01101011 01100101 01100101 01110000")) {
    boolean contains = false;
    for (Timeout timeout : connections) {
        if (timeout.getAddress().equals(connection.getOtherEnd().getAddress())) {
            contains = true;
            timeout.setTimerTask(new TimerTask() {
                @Override
                public void run() {
                    connections = ArrayModification.remove(connections, timeout);
                    System.out.println("Client disconnected");
                }
            });
            timeout.startTimer();
            break;
        }
    }
    if(!contains) {
        Timeout timeout = new Timeout(connection.getOtherEnd().getAddress());
        timeout.setTimerTask(new TimerTask() {
            @Override
            public void run() {
                connections = ArrayModification.remove(connections, timeout);
                System.out.println("Client disconnected");
            }
        });
        connections = ArrayModification.append(connections, timeout);
        System.out.println("Client connected");
    }
}

With this new setup, everything works as it should!

Answer 2

So, you

• Create a new instance of Timer
• If the if condition is true, you cancel that instance and create another one
• Then you schedule the task, but you maintain no reference to the instance which was created, so there's no way to reference it again to cancel it...?

So, at the end of the method, you ALWAYS create and schedule ANOTHER Timer, to which you have no reference to cancel in the future.

The problem is, I think what you want is a Timer per connection. This would suggest that you might want to associate the timer with the connection object itself, but the code you've supplied is out of context so it makes it difficult to ascertain directly what might be a suitable solution

Answer 3

It seems you override timeoutTimer with another instance of Timer. So you don't call cancel() on your initial Timer you scheduled the TimerTask, you call it on a newly created timer.