Skip to content

Add LeakDetector to FlowController's deinit#226

Merged
seppesnoeck merged 4 commits intomainfrom
detect-leak-flowcontroller
Feb 6, 2023
Merged

Add LeakDetector to FlowController's deinit#226
seppesnoeck merged 4 commits intomainfrom
detect-leak-flowcontroller

Conversation

@seppesnoeck
Copy link
Contributor

This PR will detect leaks in the FlowController when it's detached.

@seppesnoeck seppesnoeck self-assigned this Dec 7, 2022

deinit {
flows.forEach(detach)
LeakDetector.detect(self)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are inside deinit here, this means FlowController is already being deallocated. Please add LeakDetector.detect(flowController) to Flow's deinit instead. Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doing this, I get a compile error: Cannot find 'flowController' in scope
That's why I went for self.

Or should I add LeakDetector.detect(flowController) to the deinit in AbstractFlow.swift?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that is what I meant by the Flow's deinit 👍 Sorry for not being explicit about the the AbstractFlow type. My apologies.

Copy link
Contributor

@tinder-cfuller tinder-cfuller left a comment

Choose a reason for hiding this comment

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

Great!

@seppesnoeck seppesnoeck merged commit b2550bd into main Feb 6, 2023
@seppesnoeck seppesnoeck deleted the detect-leak-flowcontroller branch February 6, 2023 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

framework Framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants