AsyncIterator icon indicating copy to clipboard operation
AsyncIterator copied to clipboard

Discussion: Destroy propagation in callback-based TransformIterator

Open rubensworks opened this issue 3 years ago • 2 comments

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.

rubensworks avatar Aug 18 '22 11:08 rubensworks

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

RubenVerborgh avatar Aug 22 '22 20:08 RubenVerborgh

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

rubensworks avatar Aug 23 '22 10:08 rubensworks

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?

RubenVerborgh avatar Nov 11 '22 14:11 RubenVerborgh

Perhaps a destroyed event is what we need then?

Yes, I think that would help for these cases.

rubensworks avatar Nov 14 '22 09:11 rubensworks

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

RubenVerborgh avatar Nov 14 '22 11:11 RubenVerborgh