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

smart_next_channel_number #404

Closed
wants to merge 1 commit into from
Closed

smart_next_channel_number #404

wants to merge 1 commit into from

Conversation

thisloginalsotaken
Copy link

this patch fixes issue, when _next_channel_number() cannot return channel number of previously closed channel, if there are opened channels with greater numbers.
this can lead to exhaustion of channel numbers pool for connection that actively opens and closes channels.

…nnel number of previously closed channel, if there are opened channels with greater numbers. this can lead to exhaustion of channel numbers pool for connection that actively opens and closes channels.
@gmr
Copy link
Member

gmr commented Oct 21, 2013

What is your workflow that you have > MAXINT channels in a single connection? Seems like an odd condition to run into.

@thisloginalsotaken
Copy link
Author

it is not MAXINT, channel number is passed to server as short, so is is
capped at 2*16.
i have a situation where i have a running server that sometimes open new
channels. and sometimes closes old channels. closed channels typically have
lower channel numbers. so after some time next proposed channel number
exceeds 65535 and *I
get an error, event if a number of simultaneously
opened channels is relatively small.

so suppose I opened channel number 1, then number 2, then number 3 and then
closed number 1
next channel number is going to be 4, even there are only two opened
channels. after some time this number grows to 65536 and I get an error.

2013/10/21 Gavin M. Roy notifications@github.com

What is your workflow that you have > MAXINT channels in a single
connection? Seems like an odd condition to run into.


Reply to this email directly or view it on GitHubhttps://github.com//pull/404#issuecomment-26734138
.

@gmr
Copy link
Member

gmr commented Oct 22, 2013

My bad, yes not MAXINT but it seems like the easier fix is just to wrap and check if the value is used and open, no?

@gmr
Copy link
Member

gmr commented Apr 12, 2014

I appreciate the patch and will steal your tests, but will move to a list comprehension for performance reasons. Lines 1128 through 1148 can be reduced to:

[x + 1 for x in sorted(self._channels.keys() or [0]) if x + 1 not in self._channels.keys()][0]

@gmr
Copy link
Member

gmr commented Apr 12, 2014

Tracking in issue #460

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 this pull request may close these issues.

None yet

2 participants