Do I need to synchronize this? If so, how to do it?

Question

I have a simple server task that needs to listen for packets and save them every so often. This is the code I am using:

class ServerTask implements Runnable {

    private final Socket client;
    private final Timer timer;
    private Packet lastPacket;

    public ServerTask(Socket client) {
        this.client = client;
        this.timer = new Timer();

        timer.scheduleAtFixedRate(
            new TimerTask() {
                @Override
                public void run() {
                    saveLastPacketToLog();
                }
            }, 60*1000, 60*1000);
    }

    public void run() {
        try (ObjectInputStream fromClient = new ObjectImputStream(client.getOutputStream())) {
            while (running) {
                lastPacket = (Packet) fromClient.readObject();
            }
        } catch (Exception e) {
            ... 
        }
    }

    public void saveLastPacketToLog() {
        // ... open file, serialize lastPacket and write it there
    }
}

Now, since the Timer uses another Thread, maybe I should sync the access to lastPacket. Do I have to do that? If so, how to do that? I had though about doing this, but I am not sure because i have not much experience with multithreading:

class ServerTask implements Runnable {

    private final Socket client;
    private final Timer timer;
    private Packet lastPacket;

    public ServerTask(Socket client) {
        this.client = client;
        this.timer = new Timer();

        timer.scheduleAtFixedRate(
            new TimerTask() {
                @Override
                public void run() {
                    saveLastPacketToLog();
                }
            }, 60*1000, 60*1000);
    }

    public void run() {
        try (ObjectInputStream fromClient = new ObjectImputStream(client.getOutputStream())) {
            while (running) {
                synchronized(this){
                    lastPacket = (Packet) fromClient.readObject();
                }

            }
        } catch (Exception e) {
            ... 
        }
    }

    public synchronized void saveLastPacketToLog() {
        // ... open file, serialize lastPacket and write it there
    }
}

Answer 1

Do I have to [sync the access to lastPacket]?

No, you do not have to do that: since you are interested in writing out the last packet approximately as of the time the timer has fired, and because the receiver thread does not mutate an existing packet, the synchronization is unnecessary.

Making lastPacket volatile should be sufficient to implement the logic:

private volatile Packet lastPacket;

Note: the above assumes that fromClient.readObject(); returns an entirely new object, which is independent of the lastPacket object.

Answer 2

use a new Thread or a Thread from a pool when each request come,