Skip to content

Conversation

@yihong0618
Copy link
Contributor

@yihong0618 yihong0618 commented Oct 27, 2025

the count should not be *count = channels->numopen;
because the channel may close after it close the ref->chan will be NULL ref->chan == NULL

Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@yihong0618 yihong0618 changed the title fix: can not list_all after _interpchannels.close gh-140652: fix can not list_all after _interpchannels.close Oct 27, 2025
@efimov-mikhail
Copy link
Member

efimov-mikhail commented Oct 27, 2025

I'm not an expert of this code.
But IMO it'd be better to return all channel ids here, and just skip the defaults.
I.e. to process logic like this:

        ids[i] = (struct channel_id_and_info){
            .id = ref->cid,
            .defaults = (struct _channeldefaults) {
                .unboundop = -1,
                .fallback = -1,
            },
        };
        if (ref->chan != NULL) {
            ids[i].defaults = ref->chan->defaults;
        }

For example, you can see clean_up_channels method at Lib/test/test__interpchannels.py:

def clean_up_channels():
    for cid, _, _ in _channels.list_all():
        try:
            _channels.destroy(cid)
        except _channels.ChannelNotFoundError:
            pass  # already destroyed

This method assumes that ids of closed channels present in _channels.list_all().

@efimov-mikhail efimov-mikhail added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Oct 27, 2025
@yihong0618
Copy link
Contributor Author

list_all

follow the doc

I think list_all return active channels so do not return closed channels is right.

PyDoc_STRVAR(channelsmod_list_all_doc,
"channel_list_all() -> [cid]\n\
\n\
Return the list of all IDs for active channels.");

@efimov-mikhail
Copy link
Member

I'm not sure about it. We need someone else's opinion.

cc @ericsnowcurrently

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants