RubenVerborgh / AsyncIterator

An asynchronous iterator library for advanced object pipelines in JavaScript
https://rubenverborgh.github.io/AsyncIterator/docs/
Other
48 stars 7 forks source link

Discussion: Destroy propagation in callback-based TransformIterator #86

Closed rubensworks closed 2 years ago

rubensworks commented 2 years ago

I don't think it's really a bug, but nevertheless, I think it deserves some discussion.

Yesterday, I discover some edge-cases in which chained iterators would keep on running (due to an infinite iterator in the chain), even though the head of the chain was destroyed. While destroy calls should be propagated (unless destroySource is set to false), this was not happening in this case.

The root cause of these problems seemed to be caused by TransformIterators with an (async) callback-based reference to another iterator, where this iterator was created externally.

They shared the following form:

const baseIterator = ...; // Some iterator that has to be created beforehand, and can not be placed in the async callback below.

const transformIt = new TransformIterator(async() => baseIterator.map(...), { autoStart: false });

In cases where transformIt (or some other iterator in the chain from transformIt) was destroyed before it was being read, baseIterator would not be destroyed. This makes sense of course, because the callback in the TransformIterator is never invoked, which breaks the destroy call chain baseIterator.

I've created a workaround for these cases, so the problem is solved on my end. But I thought it would be good to raise this issue here as well. Perhaps it can help someone with debugging in the future.

RubenVerborgh commented 2 years ago

Thanks for reporting; however, I don't fully understand the case yet. What is the relationship between baseIterator and transformIt?

rubensworks commented 2 years ago

Ah, apologies, there was a typo in the example. Updated now, should make more sense :-)

RubenVerborgh commented 2 years ago

It's a matter of ownership indeed; the baseIterator was created outside of the TransformIterator, and ownership was never transfered.

Perhaps a destroyed event is what we need then?

rubensworks commented 2 years ago

Perhaps a destroyed event is what we need then?

Yes, I think that would help for these cases.

RubenVerborgh commented 2 years ago

Super, following up in https://github.com/RubenVerborgh/AsyncIterator/issues/95