Skip to content
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

RUM-4821 unregister vital listeners when view is stopped #2009

Merged
merged 5 commits into from
Apr 26, 2024

Conversation

xgouchet
Copy link
Member

What does this PR do?

This fixes a bug where we forgot to unregister the vital listeners. This lead to two issues:

  • we created a memory leak, with listeners references being kept in the monitor and never cleaned up
  • when a view would wait for event to finish and send its last update, vital value could be skewed because of what happened afterwards.

Fixes #2008

@xgouchet xgouchet requested review from a team as code owners April 24, 2024 20:34
@codecov-commenter
Copy link

codecov-commenter commented Apr 24, 2024

Codecov Report

Attention: Patch coverage is 94.87179% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 83.69%. Comparing base (cff8b54) to head (6567e67).
Report is 2 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2009      +/-   ##
===========================================
- Coverage    83.71%   83.69%   -0.01%     
===========================================
  Files          488      488              
  Lines        17736    17772      +36     
  Branches      2659     2667       +8     
===========================================
+ Hits         14846    14874      +28     
+ Misses        2177     2172       -5     
- Partials       713      726      +13     
Files Coverage Δ
...ndroid/rum/internal/domain/scope/RumActionScope.kt 97.77% <100.00%> (+0.61%) ⬆️
.../android/rum/internal/domain/scope/RumViewScope.kt 94.81% <94.44%> (+0.46%) ⬆️

... and 26 files with indirect coverage changes

@xgouchet xgouchet requested a review from 0xnm April 25, 2024 15:59
@xgouchet xgouchet force-pushed the xgouchet/RUM-4281/vital_memory_leak branch from 8e3bc63 to 6567e67 Compare April 26, 2024 08:59
@xgouchet xgouchet merged commit 0c3e683 into develop Apr 26, 2024
21 checks passed
@xgouchet xgouchet deleted the xgouchet/RUM-4281/vital_memory_leak branch April 26, 2024 10:48
@xgouchet xgouchet added this to the 2.9.X milestone Jul 31, 2024
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.

Memory Leak : AggregatingVitalMonitor listeners are never unregistered
4 participants