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

Fixes an issue where holding self weakly is causing a memory leak when calling workflow.abandon #190

Conversation

nickkaczmarek
Copy link
Contributor

Abstract

There was an issue where calling workflow.abandon in the onFinish completion handler of a launched workflow could fail to be called because the workflow had already been deallocated. This caused ViewControllers to remain allocated. This issue did not occur when the workflow was abandoned within the workflow, but did reliably occur when trying to clean up when finishing.

By making this a strong reference, we can see in the memory graph that the ViewControllers are being reliably deallocated. I attached a video of me doing just this. Starting with the current code and showing all of the ViewControllers still in memory and then re-running with the strong reference showing all of the ViewControllers no longer in memory.

Video

video showing memory graph debugging

Linked Issue: Closes #41

Checklist:

…xes an issue where holding self weakly is causing a memory leak when calling workflow.abandon - nk
@nickkaczmarek nickkaczmarek added the bug Something isn't working label Mar 8, 2022
@nickkaczmarek nickkaczmarek self-assigned this Mar 8, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #190 (caca894) into main (edaedd0) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #190   +/-   ##
=======================================
  Coverage   91.28%   91.28%           
=======================================
  Files          92       92           
  Lines        2375     2375           
=======================================
  Hits         2168     2168           
  Misses        207      207           
Impacted Files Coverage Δ
...rent_UIKit/Extensions/WorkflowUIKitAdditions.swift 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a6b7b22...caca894. Read the comment docs.

@morganzellers morganzellers moved this from In progress (NaN) to Review in SwiftCurrent Kanban Mar 14, 2022
@morganzellers morganzellers merged commit f69e8ac into main Mar 14, 2022
@morganzellers morganzellers deleted the workflow-abandon-is-causing-memory-self-when-weakly-referenced branch March 14, 2022 18:32
@morganzellers morganzellers moved this from Review to Ready for Demo in SwiftCurrent Kanban Mar 15, 2022
@morganzellers morganzellers moved this from Ready for Demo to Done in SwiftCurrent Kanban Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging this pull request may close these issues.

_abandon on Workflow not actually called.
3 participants