Skip to content

fix(apple): Prevent dupe resources timer scheduling#8066

Merged
jamilbk merged 2 commits into
mainfrom
fix/prevent-dupe-timers
Feb 9, 2025
Merged

fix(apple): Prevent dupe resources timer scheduling#8066
jamilbk merged 2 commits into
mainfrom
fix/prevent-dupe-timers

Conversation

@jamilbk

@jamilbk jamilbk commented Feb 9, 2025

Copy link
Copy Markdown
Member

If we receive two .connected status change updates from the system in a row, we'll incorrectly schedule an additional timer, and the handle to the first one will be lost.

This causes a memory leak because we'll then never call the first timer's invalidate() function in the endUpdatingResources call.

@vercel

vercel Bot commented Feb 9, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
firezone ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 9, 2025 10:59pm

@jamilbk jamilbk changed the title fix(apple): Prevent duplicate updatingResources timer scheduling fix(apple): Prevent dupe resources timer scheduling Feb 9, 2025
func beginUpdatingResources(callback: @escaping (ResourceList) -> Void) {
Log.log("\(#function)")

guard self.resourcesTimer == nil

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems weird to me to use a guard without actually creating a new variable that holds the state you are guarding. If you don't need the resource timer, just make it an if != nil.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@jamilbk
jamilbk enabled auto-merge February 9, 2025 22:57
@jamilbk
jamilbk added this pull request to the merge queue Feb 9, 2025
Merged via the queue into main with commit 36f5eee Feb 9, 2025
@jamilbk
jamilbk deleted the fix/prevent-dupe-timers branch February 9, 2025 23:28
@sentry

sentry Bot commented Feb 11, 2025

Copy link
Copy Markdown

Sentry Issue: APPLE-CLIENT-29

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.

2 participants