lyft / Kronos-Android

An Open Source Kotlin SNTP library
Apache License 2.0
254 stars 19 forks source link

Remove initialization state from the SNTP service #51

Closed arturdryomov closed 3 years ago

arturdryomov commented 3 years ago

Upstreaming an internal change authored by @buildbreaker (internal #60231).

The app does a sync almost immediately after app launch. The state is almost always set to SYNCING before the get is called. This means that the INIT comparison is always false so the boot time check will never be triggered.

This should fix a large class of issues related to NTP. Specifically, if the user restarts their phone and then opens the app in poor network connectivity, they'll likely fail all the NTP requests and will be stuck using the persisted NTP time. Since the user restarted their app, the calculations for response age would be completely off (since the current elapsed ticks would always be lower than the elapsed ticks in the persistence).

Finally, we only do NTP syncs on an app open so if this were to trigger on a specific session, the rest of the session's timestamps would be off too.

PTAL @buildbreaker, @ameliariely, @amphora001