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

EventLoopScheduler: set _nextItem to null on Tick #325

Closed
wants to merge 2 commits into from
Closed

EventLoopScheduler: set _nextItem to null on Tick #325

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Feb 10, 2017

This fixes a potential memory leak.

If _queue is empty after the item is removed from it, then _nextItem is never overriden and therefore keeps a reference to item.
Until another action is scheduled with a future due time, the item and all its associated state is kept in memory.

If _queue is empty after the item is removed from it, then _nextItem is never overriden and therefore keeps a reference to item.
Until another action is scheduled with a future due time, the item and all its associated state is kept in memory.
@dnfclas
Copy link

dnfclas commented Feb 10, 2017

Hi @TobBrandt-Work, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, DNFBOT;

@bartdesmet
Copy link
Collaborator

@TobBrandt-Work, thanks for your contribution. We're looking into adding this to Rx 4.0 but there seems to be a merge conflict. If you have time to help to resolve this, feel free to do so. If you don't, that's fine too, and we'll have a look over here. The CI issue should have been resolved by now, so once the merge conflict goes away, I'd expect everything to turn green.

@ghost
Copy link
Author

ghost commented Aug 28, 2017

I have merged in the changes from upstream.

@clairernovotny
Copy link
Member

@TobBrandt-Work Can you please rebase this off of develop? It should not be going into master.

Thanks

@clairernovotny clairernovotny changed the base branch from master to develop August 29, 2017 02:24
@ghost
Copy link
Author

ghost commented Aug 29, 2017

I cannot change the source branch of the PR (at least I don't know how).

New PR: #421

@ghost ghost closed this Aug 29, 2017
This pull request was closed.
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.

3 participants