Skip to content

Conversation

@aleksandr-gringauz
Copy link
Contributor

@aleksandr-gringauz aleksandr-gringauz commented Nov 19, 2025

What does this PR do?

See the ticket for a stacktrace.

I haven't managed to determine how widespread this crash is, I can't find it in the list of crashes on google play console, I only can see it through the link shared by a single customer though google play console.

I haven't managed to reproduce it either.

The crash happens most likely because (it is the most plausible explanation I can come up with):

  1. We call kronosClock.shutdown here.
  2. After that kronosClock.getCurrentTimeMs is called as a result of this call to sdkCore.time.deviceTimeNs.
  3. This ensureServiceIsRunning call throws IllegalStateException here.
  4. We interact with already shutdown kronos clock, because this sdkCore.time.deviceTimeNs can get the old timeProvider, after that kronosClock.shutdown is called on another thread and finally sdkCore.time.deviceTimeNs interacts with this clock using the old 'timeProvider`.

This PR does the following:
Add try/catch in KronosTimeProvider so that it becomes physically impossible to crash if by some reason we interact with an already shutdown kronos clock.

We could make timeProvider volatile. But it will not help with the point 4. sdkCore.time.deviceTimeNs is not "atomic", clock can be shut down at any point during execution of this code, so the best we can do is try/catch.

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@aleksandr-gringauz aleksandr-gringauz force-pushed the aleksandr-gringauz/RUM-11821/fix-kronos-crash branch from ba267f4 to effd411 Compare November 19, 2025 12:55
@codecov-commenter
Copy link

codecov-commenter commented Nov 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.15%. Comparing base (88d29ba) to head (90f2151).
⚠️ Report is 4 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3014      +/-   ##
===========================================
- Coverage    71.18%   71.15%   -0.04%     
===========================================
  Files          859      859              
  Lines        31358    31373      +15     
  Branches      5281     5285       +4     
===========================================
- Hits         22322    22321       -1     
- Misses        7539     7552      +13     
- Partials      1497     1500       +3     
Files with missing lines Coverage Δ
...n/com/datadog/android/core/internal/CoreFeature.kt 85.34% <100.00%> (ø)
...g/android/core/internal/time/KronosTimeProvider.kt 100.00% <100.00%> (ø)

... and 29 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@aleksandr-gringauz aleksandr-gringauz force-pushed the aleksandr-gringauz/RUM-11821/fix-kronos-crash branch from effd411 to 4148140 Compare November 19, 2025 14:10
@aleksandr-gringauz aleksandr-gringauz marked this pull request as ready for review November 19, 2025 16:44
@aleksandr-gringauz aleksandr-gringauz requested review from a team as code owners November 19, 2025 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants