Skip to content

Comments

Fix: Add missing [weak self] in ServiceAlertViewController async closures#1053

Merged
aaronbrethorst merged 1 commit intoOneBusAway:mainfrom
diveshpatil9104:fix/service-alert-weak-self-async-closures
Feb 19, 2026
Merged

Fix: Add missing [weak self] in ServiceAlertViewController async closures#1053
aaronbrethorst merged 1 commit intoOneBusAway:mainfrom
diveshpatil9104:fix/service-alert-weak-self-async-closures

Conversation

@diveshpatil9104
Copy link
Contributor

@diveshpatil9104 diveshpatil9104 commented Feb 18, 2026

Summary

ServiceAlertViewController captures self strongly in two async closures, preventing deallocation if the user navigates away before the page finishes rendering.

Problem

preparePage() dispatches work to a background queue and displayPage() dispatches back to the main queue — both capture self strongly. If the user opens a service alert and navigates back quickly, the view controller stays alive in memory until both closures complete.

Fix

Added [weak self] to both closures:

  • preparePage() — background queue closure
  • displayPage() — main queue closure with guard let self before accessing properties

Why this matters

This follows the convention already used throughout the codebase where async closures always use [weak self].

Copy link
Member

@aaronbrethorst aaronbrethorst left a comment

Choose a reason for hiding this comment

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

Nice catch, Divesh — this is a clean and correct fix for a real memory management issue. The [weak self] pattern is exactly the right approach here, and it's consistent with how async closures are handled throughout the rest of the codebase.

@aaronbrethorst aaronbrethorst merged commit d7f2508 into OneBusAway:main Feb 19, 2026
2 checks passed
@diveshpatil9104 diveshpatil9104 deleted the fix/service-alert-weak-self-async-closures branch February 19, 2026 06:57
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