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

Correct leaks on Switch and Divider on iOS. #2850

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

freakboy3742
Copy link
Member

@freakboy3742 freakboy3742 commented Sep 16, 2024

The Switch and Divider widgets on iOS were keeping hard references to the underlying impl/interface, causing leaks.

The same fix should be possible on Box as well... but making the change causes a segfault on OptionContainer and ScrollContainer tests. The fix wasn't obvious; I figured it's better to live with the existing leak for now, and fix the leak where we know it exists. There's likely some overlap with the consequences of beeware/rubicon-objc#256.

Refs #2849.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

Copy link
Member

@samschott samschott left a comment

Choose a reason for hiding this comment

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

Thanks!

@freakboy3742 freakboy3742 merged commit 3e503e8 into beeware:main Sep 16, 2024
38 checks passed
@freakboy3742 freakboy3742 deleted the ios-leaks branch September 16, 2024 08:43
@samschott
Copy link
Member

Out of curiosity, what did you try to fix the leaks on the iOS container widgets? Is the issue there that the parent UIView maintains a strong reference to its children?

@freakboy3742
Copy link
Member Author

I didn't get very far - the object that was causing the segfault wasn't immediately obvious. The manifestation is that switching the implementation of Box from a raw UIView to TogaView causes a bunch of the OptionContainer and ScrollContainer tests to segfault. They all seem to fail in post-test cleanup; and they all seem to be related to tests that replace views during the test run - but because it's a segfault during garbage collection in a test cleanup, it's difficult to narrow down the source (if you've got any hot tips on narrowing down the object - or even the line of code - that causes a segfault, I'd love to hear them...)

Thinking about it with a fresh mind - it might be related to the constraints. There's a specific protection in the cocoa constraints implementation that doesn't exist in the iOS implementation... I'll try to see if porting that fix that works.

@freakboy3742
Copy link
Member Author

Confirmed - it was the constraint handling. PR incoming.

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