> After a server-timeout the client stream is corrupted. It reproduces. A stub server that answers the first request later than the client's `IOTimeout` is enough - no debugger and no genuinely slow server needed. Same result in every run on aarch64-win64 and aarch64-linux, trunk `695611fdb3`, where `fphttpclient.pp` is byte-identical to current `main` (last changed in `99148c28`). So I could not confirm that it is Windows-specific.
With `KeepConnection := True`, a read timeout raises
`EHTTPClientSocketRead` (`fphttpclient.pp:1014`, `:1379`).
`DoKeepConnectionRequest` catches it at `:1672`. When it retries,
`ReconnectToServer` (`:1695`) closes the socket properly. When the
reconnect limit is exhausted it re-raises at `:1675` without closing
anything, and the `finally` at `:1700` disconnects only on
`HasConnectionClose or Terminated` - with no `Connection` header in the
request, `HasConnectionClose` (`:915`, reading the *request* headers via
`:750`) is False. So the caller gets an exception and the client keeps a
socket whose state it does not know. The server's late response then lands
on that socket and stays queued.
Measured with `KeepConnectionReconnectLimit := 0` so that the first
failure reaches the caller directly; server delay 1500 ms, `IOTimeout`
300 ms, every reply carrying its own request path. In short:
```
GET /one -> EHTTPClientSocketRead (expected)
client still connected: yes (this is the defect)
GET /two -> "reply=1 path=/one"
```
The default limit of 1 arrives at the same place, it just needs the second
failure: in a POST run the socket was still assigned after the exception
had reached the caller.
Three of six Windows runs showed something extra. The body was
`reply=1 path=/one` immediately followed by the *whole* second response,
status line and headers included. That is `:1530`-`:1534`: whatever the
header read already pulled into the buffer is written to the caller's
stream in full, before `Content-Length` is looked at (`:1537`). The reads
that follow do respect the remaining length (`:1541`-`:1549`), but that
first write is not clipped. So on a keep-alive connection carrying more
than one queued response the caller can get too much data, not merely the
wrong data.
Adding a `DisconnectFromServer` to the handler at `:1672` removes the
corruption - `/two` then gets `reply=2 path=/two`, on both platforms. It may
belong in the `finally` instead, or `ReadResponse` may be the right place to
close on error; that is a call for whoever owns the unit.
That leaves the retry, and the test grew scenarios while I looked at it,
because the first answer I tried was wrong.
The obvious one - propagate `EHTTPClientSocketRead` instead of retrying it -
also throws away the recovery from an abortive close (`SO_LINGER` zero, then
close), which arrives as exactly the same exception. Only a *graceful* close
avoids the handler altogether: `recv` returns 0, `FillBuffer` leaves through
`Exit(False)` (`:1012`), `ReadResponse` returns False (`:1517`), and the
retry runs through `SkipReconnect`. Scenarios 3 and 4 measure the two apart.
Which then turned up two more things, both on the stock tree:
- The re-sending has nothing to do with timeouts. When the server closes the
connection gracefully as a POST arrives, the reconnect at `:1695` repeats
it without asking what method it is. The stub server parsed the order
twice and the caller got a 200 and no hint at all (scenario 5).
- A response that had already started is not discarded. Half a body arrives,
the read then fails, and the retry appends the whole answer to what is
already in the caller's stream - `"reply=2 pareply=3 path=/second"`
(scenario 6). The body bytes go into the caller's stream directly
(`:1534` for what the header read had already buffered, `:1386` for the
rest) and the retry reuses that stream without rewinding it. I trigger
the failure with a reset there rather than a stall, to keep the timing
out of it; the failure class is the same one the timeout produces.
Failed checks out of fourteen, three runs each, aarch64-win64 /
aarch64-linux, identical on both:
```
unchanged 6 6
+ DisconnectFromServer at :1672 3 3
+ also never retry a read error 4 4 (+1 inconclusive)
+ instead: retry only idempotent methods 2 2
```
The third line loses the three checks of the reset scenario, and scenario 6
then has nothing to look at, which is the inconclusive one.
The last line is `if not IsIdempotent(AMethod) or <limit reached> then raise`
in the handler, along the lines of RFC 9110 9.2.2. It does not reach the
other two, which sit on the `SkipReconnect` path and in `ReadResponse`.
What you reported is the first scenario, and the second line of the table is
that scenario passing. The three that remain there are the retry findings
above; the fourth line trades one of them for a behaviour change I have no
standing to argue for. All of that is a separate matter from your report,
and I would rather not load it onto your thread - shall I file it as its own
issue?
The test is one self-contained `.pas` file, six scenarios and fourteen
checks - the stub server is raw sockets on purpose, so the only fcl-web
code under test is the client.
Sven<<attachment: fpc-httpclient-timeout-test.zip>>
_______________________________________________ fpc-pascal maillist - [email protected] https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-pascal
