streaming test

[kazu-yamamoto]

@epoberezkin I added the test case which you described in #41.

Fortunately or unfortunately, the test passes.
Would you modify this test so that it fails?

[epoberezkin]

Thanks for looking into it!

Btw, you are not calling flush in the end (that I call done, probably incorrectly:) – is it not needed?

The scenario I described here was server streaming the response to the client, not client streaming the request - sorry for the confusion.

While we are at it, you can try increasing the chunk size to 32kb - it will fail. It is probably what's agreed between client and server (or it's the default 16kb data frame limit), but the spec says it can be increased during the agreement phase - I assume it's not supported yet?

Also, I might be wrong, but it seems that currently it doesn't send frames until it's confirmed by the receiving side (both up and down, although up – to the server – is about 2x faster with local server) – it results in a bad up/download performance. If I am correct, then probably instead of waiting for the response clients and servers have to maintain a set of sent chunks, and send up to some configured max number of them without waiting for the confirmation, and then track unconfirmed chunks).

[epoberezkin]

Trying to make a failing test case... Passing so far :)

[epoberezkin]

Hm, this is really weird... I cannot reproduce it in HTTP2 test, but in my test I am observing one of three outcomes, randomly:

1. sometimes it all passes (lots of debug outputs help it somehow)
2. sometimes the client hangs with
3. sometimes the client hangs having received "exceeds maximum frame size" as the last bytes

Can it be content-related? Or maybe somehow concurrency related? Maybe a stupid question - are you certain that only one thread can read from/write to any given open socket?

And now that I removed all debug output it consistently hangs.

[epoberezkin]

can it be related to the fact that it's connected via TLS?

[epoberezkin]

Adding a small delay between reads makes it consistently pass without padding (threadDelay 100)... This is the function confReadN passed to Config - it makes buffered reads from TLS context, but this function is thread safe - it doesn't start the next read before the previous one is filled or until underlying socket returns an empty string... It is all working very reliably in vanilla TLS server.

[epoberezkin]

I know it's too much to ask, but maybe you could see what happens in my code and you'll have some idea what could be wrong. I thought that the issue might have been that we had backend that could throw exception on shorter strings passed to HTTP2 Config, so I removed it, but it had no effect. All the interesting lines are in https://github.com/simplex-chat/simplexmq/pull/663/files though:

• https://github.com/simplex-chat/simplexmq/pull/663/files#diff-00305b006c6a48ffdd9fb40c3c15dd0e34e42ec4150c02d4b3d60486bb882e62R61 - the line where I pad 16 bytes (this is used in server code) - it makes some magic, preventing http2 client from hanging when trying to receive the previous chunk (not even this 16 bytes chunk, it never gets to it).
• https://github.com/simplex-chat/simplexmq/pull/663/files#diff-4141dbd518fc7aa4ca32d988f8a9ed923ae67e6049df5c48c2559744f0f2c12bR34 - adding delay here also fixes the client problem. This function is used both in the server and in the client to receive, but in this case we have client receiving hanging – it is fixed by this delay. getBuffered returns a specific number of bytes, and it should be thread-safe because of this lock - it's also used in lots of other code
• https://github.com/simplex-chat/simplexmq/pull/663/files#diff-00305b006c6a48ffdd9fb40c3c15dd0e34e42ec4150c02d4b3d60486bb882e62R68 - this is delay between chunks on the sending side, removing it also breaks http2 on large bodies (this is used in server code).

Overall, this is some concurrency issue either in http2 or in our code, but as I wrote, the same backend is used in plain TLS server and it seems to behave ok there. Will be looking over http2 code too, my hypothesis is that the concurrency issue is there :)

[kazu-yamamoto]

Btw, you are not calling flush in the end (that I call done, probably incorrectly:) – is it not needed?

This API comes from WAI. It's not done but flush.
If you call it, the buffered data is sent to the network.

[kazu-yamamoto]

Now the test fails based on #45.

[kazu-yamamoto]

The test got failed at a27cf72.
The bug was fixed at 54a0b0e.

Now SETTINGS_MAX_FRAME_SIZE is sent according to the output buffer size.
I'm going to merge this PR tomaster.

[kazu-yamamoto]

@epoberezkin Please open a new issue if there are still bugs on master.

[kazu-yamamoto]

I will try to observe the issue of small chunks vs flush.

[epoberezkin]

thank you!

[epoberezkin]

@kazu-yamamoto simplex-chat/simplexmq#667 - both workarounds are removed now with this fix, it all seems to work locally! Will report on how it works in the field once we deploy to real servers, we've just released XFTP that uses your library yesterday: https://simplex.chat/blog/20230301-simplex-file-transfer-protocol.html

Thank you!