nats.java icon indicating copy to clipboard operation
nats.java copied to clipboard

MessageManager.lastMsgReceived uses wall clock

Open powturns opened this issue 4 months ago • 2 comments

Observed behavior

lastMsgReceived is set using System.currentTimeMillis(), as a result, it is susceptible to changes in the system clock. For example, if the system clock jumps into the future (eg: due to a ntp date sync), the system may erroneously think it has missed a heartbeat.

Expected behavior

Changing the system clock should not affect heartbeat timeouts.

This is actually more difficult than it sounds:

  1. MessageManager would need to be fixed to rely on a time source that isn't tied to the wall clock
  2. The Timer class uses System.currentTimeMillis() in its implementation, so MessageManager would need a different task scheduling API that uses a monotonically increasing time source. For example ScheduledExecutorService
  3. On platforms such as android System.nanoTime() does NOT include the time the processor was in deep sleep. Instead SystemClock.elapsedRealtime() / SystemClock.elapsedRealtimeNanos() should be used. As a result, ScheduledExecutorService may not timekeep correctly.

Server and client version

Java client 2.21.2 / any server version

Host environment

No response

Steps to reproduce

  1. Set the idle heartbeat to a sufficiently large value
  2. Receive a message
  3. Change the system time to a future time that exceeds the idle heartbeat
  4. Allow the MmTimerTask to run

This may actually be a bit tricky to reproduce as the Timer also relies on System.currentTimeMillis

powturns avatar Jun 16 '25 22:06 powturns

@powturns Take a look a this: https://github.com/nats-io/nats.java/pull/1334 I still have some timers to address, but this is the start.

scottf avatar Jun 19 '25 21:06 scottf

#1335 replaces timers

scottf avatar Jun 24 '25 00:06 scottf