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

Fix retain cycles #101

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix retain cycles #101

wants to merge 1 commit into from

Conversation

DrHazemAli
Copy link

Fix retain cycles

Resolved retain cycles by capturing self weakly in async closure and Combine sink subscription. This prevents Downloader from being held in memory due to strong references in URLSession and Combine.

Report

  • Issue: Retain cycles in Downloader class.
  • Symptoms: The Downloader instance remains in memory after its use, potentially leading to increased memory usage and resource leakage.
  • Cause: Strong references to self in asynchronous closures and Combine subscriptions.
  • Resolution: Used [weak self] to capture self weakly in both the getAllTasks closure and the Combine sink subscriber.
  • Status: Fixed in this pull.

This commit resolves the retain cycle issues, ensuring proper memory management for Downloader instances.

Analysis

The Downloader class was found to have potential retain cycles due to strong references to self within an async closure and a Combine sink subscription. These strong references prevent the Downloader instance from being deallocated, leading to memory leaks.

Problem Areas

  1. Async Closure in URLSession getAllTasks:
    The closure passed to urlSession?.getAllTasks in the initializer referenced self strongly, which could prevent the instance from being released if the closure retains it.

  2. Combine in waitUntilDone Method:
    The Combine sink subscriber in waitUntilDone held a strong reference to self, potentially causing a retain cycle as Combine retains its subscribers until they are explicitly canceled or deallocated.

Solution

To prevent retain cycles, I've modified the code to capture self weakly in both the async closure and the Combine.

Resolved retain cycles by capturing self weakly in async closure and Combine sink subscription. This prevents Downloader from being held in memory due to strong references in URLSession and Combine.
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.

None yet

1 participant