Weakly reference Activity for transaction finished callback #2203
Conversation
| if (unwrappedActivity != null) { | ||
| activityFramesTracker.setMetrics(activity, finishingTransaction.getEventId()); | ||
| } |
There was a problem hiding this comment.
I'd log in to the else branch, so people know why frames were not attached in this case.
There was a problem hiding this comment.
is still use activity , replace by unwrappedActivity?
There was a problem hiding this comment.
I think you looked at some old version of the code in this PR. It's using the unwrappedActivity now. I unintentionally commited the code I played around with to confirm the fix and then fixed with another commit.
| @@ -146,6 +147,7 @@ private void stopPreviousTransactions() { | |||
| } | |||
|
|
|||
| private void startTracing(final @NotNull Activity activity) { | |||
| WeakReference<Activity> weakActivity = new WeakReference<>(activity); | |||
There was a problem hiding this comment.
Please try to reproduce the issue with and without the fix using LeakCanary.
Upgrade leakcanary to the latest version -> https://github.com/square/leakcanary/releases/tag/v2.9.1
There was a problem hiding this comment.
Tried with 2.9.1 but it didn't find anything regardless of WeakReference or old implementation.
Codecov Report
@@ Coverage Diff @@
## main #2203 +/- ##
=========================================
Coverage 80.90% 80.90%
Complexity 3313 3313
=========================================
Files 236 236
Lines 12155 12155
Branches 1616 1616
=========================================
Hits 9834 9834
Misses 1725 1725
Partials 596 596 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| options | ||
| .getLogger() | ||
| .log( | ||
| SentryLevel.WARNING, |
There was a problem hiding this comment.
What log level should this be?
📜 Description
Wrap
ActivitywithWeakReferenceinActivityLifecycleIntegrationto not strongly hold on to it for theTransactionFinishedCallback.💡 Motivation and Context
Fixes #2201
💚 How did you test it?
Emulator
📝 Checklist
🔮 Next steps