QUICStream recreated after graceful destroy

[tegefaulkes]

Specification

Sorry but I don't have many details here. I'll have to reproduce it in a test here.

In Polykey during one of the tests where a single RPC client call was made and ends gracefully. I noticed that a 2nd stream was created and passed to the RPCServer to be handled but it never reached the handler stage. This caused an issue where the RPCServer.stop() just hung since the stream never ended.

We've seen this problem before where the quiche stream state is not fully cleaned up after the stream has ended. In this case the stream still exists on the iterator and is re-created as a QUICStream. We need to double check if this is still a problem and make sure the streams are properly cleaned up in quiche.

To properly clean up a stream in quiche you need to attempt a write that throws if its a writable. or attempt a read that throws if it is a readable. If we don't do this the state wont transition.

Minor detail, but the stream was re-created on the client connection but I didn't see it on the server connection side.

Additional context

• Related: Consolidating ClientService and js-ws and js-rpc integration Polykey#560

Tasks

1. Check if the quiche stream state has been cleaned up properly on the client connection side for a gracefully ended stream.
2. apply a fix if needed.

[CMCDragonkai]

I wonder if there's a check we can do when a stream being recreated like the new stream ID is below a counter. We have one-sided counter but we could also maintain the other sides counter so we know exactly when this occurs.

[CMCDragonkai]

Is this still a problem and should it be considered part of the stability issues?

[tegefaulkes]

I'll have to double check if this is still a problem.

[CMCDragonkai]

I think we should have an exception that throws if the same stream-id is re-used. Also stream ID is always incremented.

[tegefaulkes]

I had a look over it and added 3 tests. I don't see this problem happening currently and if it does with further changes the new test should catch it. I'm going to close this issue with the new commit. It doesn't warrant a PR.