Skip to content

MudCarousel: Fix timer creation after component disposal#11192

Merged
ScarletKuro merged 3 commits intoMudBlazor:devfrom
jHabjanMXP:fix/mud-carousel-timer-creation-after-disposal
Apr 14, 2025
Merged

MudCarousel: Fix timer creation after component disposal#11192
ScarletKuro merged 3 commits intoMudBlazor:devfrom
jHabjanMXP:fix/mud-carousel-timer-creation-after-disposal

Conversation

@jHabjanMXP
Copy link
Contributor

Issue

Found memory issues:
image
image
image
See the FromSeconds 5 which is referenced in the graph
image
image
Timer is sometimes created after dispose is called

Changes

  • added _disposing to keep track of disposed state
  • not creating timer if disposing is set to true

Reason

  • If component is created, and then immediately destroyed, timer may still get created in OnAfterRenderAsync lifecycle hook, which would cause timer to be created after disposal, which in turn leads to memory leak

How Has This Been Tested?

Added Console.WriteLine when timer should be created or reset

Type of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (fix or improvement to the website or code docs)

Checklist

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

## Changes
- added `_disposing` to keep track of disposed state
- not creating timer if disposing is set to true

## Reason
- If component is created, and then immediately destroyed, timer may still get created in OnAfterRenderAsync lifecycle hook, which would cause timer to be created after disposal, which in turn leads to memory leak
## Changes
- added condition to not reset timer when disposing
@github-actions github-actions bot added bug Unexpected behavior or functionality not working as intended PR: needs review labels Apr 14, 2025
@codecov
Copy link

codecov bot commented Apr 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.07%. Comparing base (df922eb) to head (f805459).
Report is 1 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #11192      +/-   ##
==========================================
- Coverage   91.08%   91.07%   -0.01%     
==========================================
  Files         436      436              
  Lines       14110    14110              
  Branches     2731     2731              
==========================================
- Hits        12852    12851       -1     
  Misses        640      640              
- Partials      618      619       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ScarletKuro ScarletKuro merged commit f1c4667 into MudBlazor:dev Apr 14, 2025
2 checks passed
@sonarqubecloud
Copy link

@jHabjanMXP jHabjanMXP deleted the fix/mud-carousel-timer-creation-after-disposal branch April 15, 2025 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Unexpected behavior or functionality not working as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants