-
-
Notifications
You must be signed in to change notification settings - Fork 435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix crash accessing Choreographer instance #2970
Conversation
…a crash reported by Google Play SDK console
Performance metrics 🚀
|
Codecov ReportAttention:
📢 Thoughts on this report? Let us know!. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, please check my comment!
.post( | ||
() -> { | ||
try { | ||
choreographer = Choreographer.getInstance(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since choregrapher is accessed via a handler outside the main thread I think it's a good idea to mark the field as volatile
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, since it's only set once and read multiple times. We'd lose memory cache for that.
In the worst case now, when it's read it's still null, so the frame start will fallback to now - frameDuration
📜 Description
choreographer instance is now retrieved in a try/catch block, due to a crash reported by Google Play SDK console
💡 Motivation and Context
Fixes a crash. Closes #2912
💚 How did you test it?
📝 Checklist
sendDefaultPII
is enabled.🔮 Next steps