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 leak of some associated properties in AutoLayout.mm #1461

Merged
merged 1 commit into from
Nov 28, 2016

Conversation

ehren
Copy link
Contributor

@ehren ehren commented Nov 27, 2016

Missing some releases.

Note: original pull also included calls to explicitly break the associations (by calling objc_setAssociatedObject with nil on autoLayoutDealloc), citing this page
https://web.archive.org/web/20120818164935/http://developer.apple.com/library/ios/#/web/20120820002100/http://developer.apple.com/library/ios/documentation/cocoa/conceptual/objectivec/Chapters/ocAssociativeReferences.html

However, as verified here https://github.com/Microsoft/WinObjC/blob/812d6636036a8d2eaeaaf7b92aa0c7b76252e190/tests/unittests/Starboard/objcrt_assoc.mm#L146 (and by manual testing) it's not necessary to manually nil out the associated objects with an objc_setAssociatedObject call.


This change is Reviewable

@ehren ehren force-pushed the autolayout-associated-leak branch from c415cfe to ab7c99c Compare November 27, 2016 20:54
@ehren ehren changed the title Fix leaks associated with objc_setAssociatedObject use in AutoLayout.mm Fix leak of some associated properties in AutoLayout.mm Nov 27, 2016
@jaredhms jaredhms self-assigned this Nov 28, 2016
@oliversa-msft
Copy link
Contributor

:shipit:

1 similar comment
@jaredhms
Copy link
Contributor

:shipit:

@jaredhms
Copy link
Contributor

In CI build.

@jaredhms jaredhms merged commit 704d169 into microsoft:develop Nov 28, 2016
ehren added a commit to ehren/WinObjC that referenced this pull request Nov 30, 2016
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.

5 participants