ninenines / cowlib

Support library for manipulating Web protocols.
ISC License
279 stars 173 forks source link

Add a function to check remote HTTP/2 setting MAX_CONCURRENT_STREAMS #123

Open zuiderkwast opened 2 years ago

zuiderkwast commented 2 years ago

This is to be used primarily by a client before initiating a new stream so that it can respect the setting from the server.

I've tested in in a draft patch to gun. Will post a gun PR soon.

essen commented 11 months ago

Right I believe the missing spec and the todo are why it didn't move forward. The todo in particular should be done. Perhaps it's better to maintain a counter in the state than count streams manually, since we want only locally initiated streams.

zuiderkwast commented 11 months ago

I did the todo and added a spec.

Perhaps it's better to maintain a counter in the state than count streams manually, since we want only locally initiated streams.

The local number of streams is never more than the total number of streams, so I added counting only if the total number reaches the limit. Thus, this is O(1) in hopefully most cases but O(n) in the worst case.

Do you prefer storing a counter in the state? There are multiple places where streams are added to and removed from the state so it would add some complexity.

essen commented 11 months ago

Probably fine like this.

zuiderkwast commented 11 months ago

Ping. This one is ready too. (Sorry for spamming.)