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

Setup a ConditionalWeakTable in ListProxy to hold weak references so that a ListView will work when connected to a Weak Source #802

Merged
merged 1 commit into from Mar 24, 2017

Conversation

Projects
None yet
6 participants
@PureWeen
Contributor

PureWeen commented Mar 7, 2017

Description of Change

If a collection implements INotifyCollectionChanged with a Weak Reference then the ListProxy's subscription will get finalized prematurely by the GC since there are no roots holding onto the WeakProxy or the CollectionChanged subscription. I added a ConditionalWeakTable keyed on the ListProxy to hold the WeakProxy and the NotifyCollectionChangedEventHandler this way when there are no more roots holding onto ListProxy they will still get disposed. All current unit tests pass and I added an additional one that demonstrates the bug.

Bugs Fixed

https://bugzilla.xamarin.com/show_bug.cgi?id=31415
I first came across this issue when trying to use a ReactiveList with Xamarin Forms
reactiveui/ReactiveUI#806

API Changes

None

Behavioral Changes

There shouldn't be any. When there are no more roots referencing the ListProxy the subscriptions will get cleaned up as is demonstrated by the unit tests.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
Setup a ConditionalWeakTable in ListProxy to hold weak references so …
…that a ListView will work when connected to a Weak Source
@dnfclas

This comment has been minimized.

Show comment
Hide comment
@dnfclas

dnfclas Mar 7, 2017

@PureWeen,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla2.dotnetfoundation.org.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
.NET Foundation Pull Request Bot

dnfclas commented Mar 7, 2017

@PureWeen,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla2.dotnetfoundation.org.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
.NET Foundation Pull Request Bot

@dnfclas dnfclas added the cla-required label Mar 7, 2017

@rmarinho rmarinho requested a review from jassmith Mar 7, 2017

@dnfclas

This comment has been minimized.

Show comment
Hide comment
@dnfclas

dnfclas Mar 7, 2017

@PureWeen, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, .NET Foundation Pull Request Bot

dnfclas commented Mar 7, 2017

@PureWeen, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, .NET Foundation Pull Request Bot

@dnfclas dnfclas added cla-signed and removed cla-required labels Mar 7, 2017

@jassmith jassmith merged commit 377d24f into xamarin:master Mar 24, 2017

7 checks passed

Android-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Tests passe…
Details
OSX-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: OSX Debug : Running
Details
Windows-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: Windows Debug : Tests passed: 3745, ignored: 10
Details
Windows-Manual-Debug Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: Windows Manual PR Review : Tests passed: 3745, ignored: 10
Details
iOS10-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 : Tests p…
Details
iOS8-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified IOS8 : Tests pa…
Details
iOS9-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS9 : Tests pa…
Details
@ghuntley

This comment has been minimized.

Show comment
Hide comment
@ghuntley

ghuntley commented Jul 4, 2017

<3

@samhouts samhouts added D-15.4 and removed cla-signed labels Oct 10, 2017

@samhouts samhouts modified the milestones: 2.3.0, 2.3.5 Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment