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

rx.disposeBag may be using wrong address #47

Closed
grinder81 opened this issue Oct 7, 2017 · 7 comments
Closed

rx.disposeBag may be using wrong address #47

grinder81 opened this issue Oct 7, 2017 · 7 comments

Comments

@grinder81
Copy link

I think rx.disposeBag is doing some wrong memory stuff. Take a look at this post. I'm investigating now but might be helpful if you take a look as well.

ReactiveX/RxSwift#1439

Check kzaher comment:

Usually what happens is that somebody is using objc_getAssociatedObject and objc_setAssociatedObject inside rx extension in this form objc_getAssociatedObject(self, ... vs the correct objc_getAssociatedObject(base, ....

Because Swift has struct reusing, those two facts combined usually results in that kind of behavior.

@ashfurrow
Copy link
Member

Yup, gotcha. That's tricky – I think you're right that the struct is getting copied and the address used by objc_get/setAssociatedObject. Hmm. We may have to remove support for non-reference-types. This was introduced in this pr. I think adding a : class constraint to the HasDisposeBag protocol should fix the issue (while removing the ability to use this on value types).

What do you think? Up for a pull request? Thanks again for opening the issue!

@grinder81
Copy link
Author

Right now I'm doing some more investigation, I'll update when I'm done but I think that's the issue.

@ashfurrow
Copy link
Member

Cool, thanks! Let us know what you find 👍

@grinder81
Copy link
Author

Ok, I did quite an investigation.

  • I removed this pod completely and made sure there is no rx.disposeBag in my code
  • Replaced rx.disposeBag by self.disposeBag

But still, I'm seeing that issue. Which means I can't blame this pod for memory leaking.

However, I think that need to be fixed. I'll try to put a PR soon.

@ashfurrow
Copy link
Member

Awesome, I look forward to it 👍

@grinder81
Copy link
Author

PR is up! Discuss and comment there.

Thanks

@ashfurrow
Copy link
Member

Closed by #48, and released to CocoaPods as 4.0.0. Thanks again!

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

No branches or pull requests

2 participants