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

(Likely) Scheduled action leaks #6561

Closed
cartant opened this issue Aug 13, 2021 · 1 comment · Fixed by #6562
Closed

(Likely) Scheduled action leaks #6561

cartant opened this issue Aug 13, 2021 · 1 comment · Fixed by #6562

Comments

@cartant
Copy link
Collaborator

@cartant cartant commented Aug 13, 2021

It seems likely to me that some - if not all - of the following scheduler.schedule calls will leak actions that are added to another subscription - similar to the leak that was fixed in #6559:

subscription.add(scheduler.schedule(execute));

subs.add(scheduler.schedule(() => emit(record), bufferTimeSpan));

subscriber.add(
scheduler.schedule(function () {
startBuffer();
!this.closed && subscriber.add(this.schedule(null, bufferCreationInterval));
}, bufferCreationInterval)
)

innerSubScheduler ? subscriber.add(innerSubScheduler.schedule(() => doInnerSub(bufferedValue))) : doInnerSub(bufferedValue);

subs.add(scheduler.schedule(() => closeWindow(record), windowTimeSpan));

subscriber.add(
scheduler.schedule(function () {
startWindow();
!this.closed && subscriber.add(this.schedule(null, windowCreationInterval));
}, windowCreationInterval)
)

sub.add(
scheduler.schedule(() => {
const iterator = input[Symbol.asyncIterator]();
sub.add(scheduler.schedule(function () {
iterator.next().then(result => {
if (result.done) {
subscriber.complete();
} else {
subscriber.next(result.value);
this.schedule();
}
});
}));
})
);

sub.add(scheduler.schedule(() => {
const observable: Subscribable<T> = (input as any)[Symbol_observable]();
sub.add(observable.subscribe({
next(value) { sub.add(scheduler.schedule(() => subscriber.next(value))); },
error(err) { sub.add(scheduler.schedule(() => subscriber.error(err))); },
complete() { sub.add(scheduler.schedule(() => subscriber.complete())); },
}));
}));

subscriber.add(
scheduler.schedule(() => {
subscriber.next(value);
subscriber.add(scheduler.schedule(() => subscriber.complete()));
})
);

const subscription = scheduler.schedule(function () {
try {
execute.call(this);
} catch (err) {
subscriber.error(err);
}
}, delay);
subscriber.add(subscription);

Some of these might leak only a few actions, but any leaked actions will have a closure that could be referencing a resource.

@benlesh

@cartant cartant changed the title Scheduled action leaks (Likely) Scheduled action leaks Aug 13, 2021
@benlesh
Copy link
Member

@benlesh benlesh commented Aug 13, 2021

I have some plans for this I think. 🤔

benlesh added a commit to benlesh/rxjs that referenced this issue Aug 13, 2021
benlesh added a commit to benlesh/rxjs that referenced this issue Aug 13, 2021
benlesh added a commit that referenced this issue Aug 14, 2021
…n references (#6562)

* fix: scheduling with Rx-provided schedulers will no longer leak action references

Resolves #6561

* refactor: Remove circular dependencies and redundant code

* chore: update api_guardian files
paulmojicatech pushed a commit to paulmojicatech/rxjs that referenced this issue Aug 17, 2021
…n references (ReactiveX#6562)

* fix: scheduling with Rx-provided schedulers will no longer leak action references

Resolves ReactiveX#6561

* refactor: Remove circular dependencies and redundant code

* chore: update api_guardian files
paulmojicatech pushed a commit to paulmojicatech/rxjs that referenced this issue Aug 17, 2021
…n references (ReactiveX#6562)

* fix: scheduling with Rx-provided schedulers will no longer leak action references

Resolves ReactiveX#6561

* refactor: Remove circular dependencies and redundant code

* chore: update api_guardian files
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 a pull request may close this issue.

2 participants