Skip to content

1.x: fix replay() retaining reference to the child Subscriber#4229

Merged
akarnokd merged 1 commit intoReactiveX:1.xfrom
akarnokd:ReplaySubscriberCacheFix
Jul 23, 2016
Merged

1.x: fix replay() retaining reference to the child Subscriber#4229
akarnokd merged 1 commit intoReactiveX:1.xfrom
akarnokd:ReplaySubscriberCacheFix

Conversation

@akarnokd
Copy link
Member

In #3470, a flat cache of Subscribers has been introduced that is versioned and gets only updated if there are new Subscribers coming, old Subscribers leaving and a value comes through - improving performance if lots of Subscribers come and go between main onNext signals. This cache is not cleared but rebuilt therefore child Subscribers unsubscribing were still referenced through it and everything beyond - leading to memory leak on sensitive systems.

The fix is to cut the link to the child Subscriber in InnerProducer on unsubscription. A secondary action also empties the cache if all Subscribers are gone.

Reported in #4228 .

@codecov-io
Copy link

codecov-io commented Jul 22, 2016

Current coverage is 84.20% (diff: 61.53%)

Merging #4229 into 1.x will increase coverage by 0.07%

@@                1.x      #4229   diff @@
==========================================
  Files           265        265          
  Lines         17305      17314     +9   
  Methods           0          0          
  Messages          0          0          
  Branches       2624       2627     +3   
==========================================
+ Hits          14559      14579    +20   
+ Misses         1893       1884     -9   
+ Partials        853        851     -2   

Powered by Codecov. Last update 479df31...0101c36

@akarnokd
Copy link
Member Author

This is a medium impact bug so it would be great if it made into tomorrow's release. /cc @artem-zinnatullin @zsxwing

@stevegury
Copy link
Member

👍

@akarnokd akarnokd merged commit 75ffcbb into ReactiveX:1.x Jul 23, 2016
@akarnokd akarnokd deleted the ReplaySubscriberCacheFix branch July 23, 2016 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants