Should we take care to remove the done callbacks when the iteration is complete? That kind of thing would be possible if __aiter__ were a generator:

async def __aiter__(self):
    while True:
        ... check _todo_left etc. ...
        yield await self._wait_for_one()
    finally:
        ... remove done callbacks off _todo futures ...

If someone calls as_completed on multiple futures and abandons iteration before the end, it would be nice to clear the callbacks. For example, if this function were called multiple times with the same set of futures, the number of callbacks on them would accumulate:

async def process(futs):
    async for fut in as_completed(futs):
        ... 
        if some_condition:
            break

@1st1 what do you think?

I think cleaning up the callbacks on iterator close makes sense. We could at least put the cleanup in __del__. If we decide to deprecate the plain iterator form, we can eventually implement the whole as_complete function as an asynchronous generator.

It should be fine to make __aiter__ return a generator because as_completed never returned an async-iterable to begin with. Regarding the implementation, note that asyncio has dedicated machinery for cleaning up async iteration which seems based on async generators and their aclose method. I'm not an expert in the area, but it's probably a better bet to make use of that mechanism than to rely on __del__.

As for __iter__, I think it's safe to leave its behavior unchanged, both for ease of implementation and to avoid affecting existing code in any way.

I think it's safe to leave its behavior unchanged, both for ease of implementation and to avoid affecting existing code in any way.

I'm ok with us doing better cleanup on the older pattern as I can't imagine any easy way for user code to have accessed or relied on the leftover callbacks, at least not with public interfaces.

It should be fine to make __aiter__ return a generator because as_completed never returned an async-iterable to begin with.

You're right, I tried generators for both __iter__ and __aiter__ following your example of using the parent iterator's _todo and was able to get it to work while maintaining backwards compatibility. I'm not keen on this approach as you end up with an extra object (the generator)

it's probably a better bet to make use of that mechanism than to rely on __del__.

I actually prefer __del__ in this case as we aren't doing any cleanup actions that require an event loop and a generator's aclose/finally requires an event loop to finalize, and as mentioned above, I like the idea of cleaning up the older style iteration as well.

Here's another quirk that comes with making __aiter__ a generator: resumption using for loops ends up iterating on a new iterator (the generator) every time. Example:

async def do_queries():
    dns_requests = asyncio.as_completed((
        resolver_a_try_ipv4,
        resolver_b_try_ipv4,
        resolver_a_try_ipv6,
        resolver_b_try_ipv6,
    ))
    async for request in dns_requests:
        result = await request
        enqueue_result_for_parsing(result)
        break

    async for request in dns_requests:
        log_slow_result(request)

    # The first generator is now garbage collected without being exhausted or aclosed.
    # CTask will throw "Task was destroyed but it is pending!" into the loop exception handler.

and if we put cleanup in the second-layer generators, it won't get called for the single iteration use pattern:

first_result = await(await asyncio.as_completed(requests).__anext__())
# Generator is never created nor is its `finally` block run.

# Or w/ plain iterator:
first_result = await next(asyncio.as_completed(requests))
# Generator is never created nor is its `finally` block run.

I do not think that __del__ works here. There are reference loops for every future: as_completed iterator -> todo set -> future -> _handle_completion bound method -> as_completed iterator. __del__ can only be called when all these loops are broken at some link, for example when the todo set is cleared, or features are cleared, or bound methods are cleared, but in this case __del__ does nothing.

Such reference loops exist in the current generator-based code too. It seems they are inevitable.