Jens-G opened a new pull request, #3782:
URL: https://github.com/apache/thrift/pull/3782
> **Stacked on #3780 and #3781.** Only the third commit belongs to this
ticket; the diff below it rebases away once those merge.
`TWebSocketServer::readFrame` answered a Ping by calling itself:
```cpp
case Opcode::Ping:
pong();
return readFrame();
```
A Ping carries nothing for the caller, so the reader has to go on to the
next frame to satisfy it. Doing that by re-entering `readFrame` costs one stack
frame per Ping, and nothing bounds how many a peer may send.
### Measured against `master`
Ping frames of one payload byte — seven bytes each on the wire — followed by
one ordinary data frame:
| pings | on the wire | stack |
|---|---|---|
| 20,000 | 140 kB | 3,200,344 bytes |
| 40,000 | 280 kB | 6,400,344 bytes |
| 60,000 | 420 kB | **segmentation fault** |
160 bytes of stack for every seven bytes of input, so the 8 MB a Linux
thread starts with is gone by about 52,000 pings. **It is not build-dependent**
— gcc 11 does not turn the call into a jump at `-O2` any more than at `-O0`,
and both builds fault at the same point. With the loop, the same run reads from
a constant depth: 416 bytes at 60,000 pings, and 416 bytes at a million, where
the wire cost is 7 MB.
### The change
A loop, and that is all. Pings are still answered one after another for as
long as a peer cares to send them, no configuration is involved, and the body
of `readFrame` is untouched apart from its indentation — `git diff -w` shows
five added lines and one changed one.
### Test
It asserts how far below its own frame the reader got, rather than waiting
for a crash: 2,000 pings — four orders of magnitude short of exhausting
anything, so it is safe in CI — take the unmodified reader 320,480 bytes deep
against a 64 kB ceiling. The fix holds it constant.
Full `bin/UnitTests`: 109 of 110. The one failure,
`TServerSocketTest/test_bind_to_address`, is a pre-existing environment failure
on this host and is present on `master`.
### Worth knowing for anyone reading this code
A Ping with **no** payload does not reach this path at all: `readFrame` does
not consume the four masking-key bytes of a zero-length masked frame, so the
next header is parsed out of the masking key and the connection dies with
*"Reserved bits must be zeroes"*. That is a separate defect, filed separately,
and it is why the numbers above use one-byte pings.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
