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

[Minor Breaking API] Make deallocation queues more reliable #651

Merged
merged 6 commits into from
Nov 2, 2017

Conversation

Adlai-Holler
Copy link
Member

In order to be sure we've released our local reference before the queue releases its reference, we need to pass a double-pointer.

Minor breaking API changes in this diff:

  • ASPerformBackgroundDeallocation(x) -> ASPerformBackgroundDeallocation(&x)
  • Swift: ASDeallocQueue.sharedDeallocationQueue() -> ASDeallocQueue.sharedDeallocationQueue
  • ASPerformMainThreadDeallocation(x) -> ASPerformMainThreadDeallocation(&x)

…n be sure we've released before the queue drains
@Adlai-Holler Adlai-Holler changed the title Make deallocation queues more reliable [Minor Breaking API] Make deallocation queues more reliable Oct 31, 2017
@TextureGroup TextureGroup deleted a comment Nov 1, 2017
@appleguy
Copy link
Member

appleguy commented Nov 1, 2017

Heh, good finds. I’m on my phone at the moment and not totally confident in my review, but I will look into it with more detail when I can (land if you get another though!)

Copy link
Member

@nguyenhuy nguyenhuy left a comment

Choose a reason for hiding this comment

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

Very nice diff!

@@ -35,7 +35,7 @@
#endif

/// For deallocation of objects on the main thread across multiple run loops.
extern void ASPerformMainThreadDeallocation(_Nullable id object);
extern void ASPerformMainThreadDeallocation(id _Nullable __strong * _Nonnull objectPtr);
Copy link
Member

Choose a reason for hiding this comment

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

Didn't know this is legit. Learning every day!

CHANGELOG.md Outdated
@@ -7,6 +7,7 @@
- [ASNetworkImageNode] New delegate callback to tell the consumer whether the image was loaded from cache or download. [Adlai Holler](https://github.com/Adlai-Holler)
- [Layout] Fixes a deadlock in layout. [#638](https://github.com/TextureGroup/Texture/pull/638) [Garrett Moon](https://github.com/garrettmoon)
- Updated to be backwards compatible with Xcode 8. [Adlai Holler](https://github.com/Adlai-Holler)
- Fixed cases where main-thread-deallocation doesn't work as expected [Adlai Holler](https://github.com/Adlai-Holler)
Copy link
Member

@nguyenhuy nguyenhuy Nov 2, 2017

Choose a reason for hiding this comment

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

Please add a quick note mentioning the API changes.

@nguyenhuy
Copy link
Member

Tested with Pinterest app and confirm that this PR fixes #607.

@Adlai-Holler Adlai-Holler merged commit ff608c9 into master Nov 2, 2017
@Adlai-Holler Adlai-Holler deleted the AHSafeAsyncDealloc branch November 2, 2017 17:45
bernieperez pushed a commit to AtomTickets/Texture that referenced this pull request Apr 25, 2018
…roup#651)

* Make our async deallocation functions take a double pointer, so we can be sure we've released before the queue drains

* Make it a class property

* Fix the return type

* Use a locker

* Improve release notes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants