nats.java
nats.java copied to clipboard
MessageManager.lastMsgReceived uses wall clock
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:
MessageManagerwould need to be fixed to rely on a time source that isn't tied to the wall clock- The
Timerclass usesSystem.currentTimeMillis()in its implementation, soMessageManagerwould need a different task scheduling API that uses a monotonically increasing time source. For exampleScheduledExecutorService - On platforms such as android
System.nanoTime()does NOT include the time the processor was in deep sleep. InsteadSystemClock.elapsedRealtime()/SystemClock.elapsedRealtimeNanos()should be used. As a result,ScheduledExecutorServicemay not timekeep correctly.
Server and client version
Java client 2.21.2 / any server version
Host environment
No response
Steps to reproduce
- Set the idle heartbeat to a sufficiently large value
- Receive a message
- Change the system time to a future time that exceeds the idle heartbeat
- Allow the MmTimerTask to run
This may actually be a bit tricky to reproduce as the Timer also relies on System.currentTimeMillis
@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.
#1335 replaces timers