UDS tests: the first 0x78 races P2ClientMax, and no test-side spacing governs it

[dborgards]

Split out of #114 because it survives that audit rather than being part of it. Not measured failing — a named quantity with a thin margin, filed so it is checkable rather than remembered.

What #117 fixed, and what it did not

#117 removed the delayBetween spacing from ResponsePending_Loop_Aborts_When_Exceeding_MaxResponsePendingCount, because that spacing raced P2StarClientMax and was not load-bearing (a no-restart mutation passes at the old 20 ms, so it carried nothing).

delayBetween only governs the gap between 0x78 responses. It says nothing about the first one, whose arrival is an ISO-TP round trip across two channels plus the simulated ECU's own loop, and which must land inside P2ClientMax or the client times out on P2 before any of the pending logic is reached.

So every one of these tests carries a first-hop budget that no test-side value controls:

test	P2ClientMax
Client_Times_Out_With_P2Star_When_Ecu_Sends_Only_ResponsePending	100 ms
ResponsePending_Restarts_P2Star_And_Returns_Final_Response	120 ms
Client_Times_Out_When_Ecu_Silent_Within_P2	150 ms
TimedOut_Request_Does_Not_Poison_Next_Same_Service_Transaction	80 ms
ResponsePending_Loop_Aborts_When_Exceeding_MaxResponsePendingCount	500 ms
two Dispose tests (:732, :760)	5 s

The bottom two are generous on purpose. The 80 ms and 150 ms entries are tests where the timeout is the subject — a slow host only makes them surer, so they fail safe. The two in bold are the exposure: the first 0x78 must arrive inside 100 ms or 120 ms, and the assertion is that something other than a P2 timeout happens.

Why this is a hypothesis and not a finding

It has never gone red, and I have not reproduced it. What makes it worth filing is the arithmetic against a quantity the host demonstrably perturbs — the same day produced three measured instances of exactly that:

	the wait	the budget	inflation measured
#115	Th 50 ms × 28 gaps	ShortTimeout 5 s	— (margin 3.6×, 6/6 red)
#116	Th 1 ms × 254 gaps	WithTimeout 10 s	Schedule(1 ms) → ~200 ms under 8× load
#117	delayBetween 20 ms	P2StarClientMax 1 s	a 20 ms Task.Delay past 1 s on macOS

A host that stretches a 20 ms Task.Delay past a second is a host that can stretch an ISO-TP round trip past 100 ms. That is the whole argument, and it is an argument, which is why this is filed rather than fixed.

What would settle it

Lower P2ClientMax in the two bold tests until they fail, under load, with the unmodified suite run as a control under the same load — the control is not optional here, and #114 records why: a probe on the ISO-TP deadlines reported one failure at every value I tried, and the control showed every one of them was an unrelated flake (Bam_MaximumPayload_Roundtrip, now #116) rather than anything the probe caused.

If the cliff is near 100 ms, these are red and want fixing. If it is an order of magnitude below, this issue closes as not planned with the number recorded.

What a fix would not be

Raising P2ClientMax. It is a product timing option, the tests set it deliberately low to prove the client is not living inside a generous initial budget (ResponsePending_Restarts_P2Star_… says so in its own comment), and raising it is the tolerance that gets widened again next time.

The real answer is the one #114's seam table already names for this layer: UdsClientImpl reads Stopwatch.GetTimestamp() directly at :804, so P2 and P2* cannot be driven by a test clock. A time seam there would remove the whole family of first-hop races at once — and it overlaps #102's area, so the two want sequencing together.

Provenance

This was written into #117's description as "it stays on #114" before any such record existed. It did not; #114 has five comments and none mentions P2ClientMax. Filed here after the maintainer asked whether an issue existed or whether I had merely noted it.

Refs #92, #114, #117.

[dborgards]

Measured. The answer is neither of the two this ticket offered

If the cliff is near 100 ms, these are red and want fixing. If it is an order of magnitude below, this issue closes as not planned with the number recorded.

The cliff is between 40 ms and 25 ms, under 8 CPU burners on 4 cores, both bold tests lowered together:

P2ClientMax	failures
60 ms	0 / 6
40 ms	0 / 6
25 ms	4 / 6
15 ms	6 / 6
10 ms	6 / 6

So the margin is 2.5–4× at 100 ms and 3–4.8× at 120 ms. Not "near 100 ms", and not an order of magnitude either — it lands between the two outcomes this ticket wrote down in advance, which is worth saying rather than rounding to whichever bucket is more convenient.

By this repository's own standard that is thin. #92's rule is that the margin has to be large compared with the perturbation, and a factor of three against a host that has already been measured stretching a 20 ms Task.Delay past a second (#117) is not large.

One failure at the stock values, and it is worthless

The first loaded probe produced 1 failure in 6 at the unmodified 100/120 ms. I did not capture it, and 64 further loaded runs at those values produced 0.

So: 1 in 70, unattributed. Under this repository's rule — a probe whose failures you did not attribute has told you nothing — it counts for nothing, and I am recording it as an unattributed event rather than as evidence for the hypothesis. The cliff is the number that stands.

Control, as this ticket required: the whole of UdsClientTests (24 tests) under the same load, unmodified, 0 failures in 2 runs. So the sweep's failures were caused by the sweep.

Recommendation: keep open, and do not touch these numbers

Raising P2ClientMax remains the wrong answer for the reason already written here. Lowering the load the test does is not available either — the first hop is an ISO-TP round trip across two channels plus the simulated ECU's loop, and that is the thing under test.

The fix this ticket already names is the right one: a time seam in UdsClientImpl, so P2 and P2* are driven by a test clock rather than Stopwatch.GetTimestamp().

I sized it and did not build it. The clock reads are few — :804 and the Elapsed helper at :968 — but measuring is the easy half: the P2 deadline is also waited on, and a seam that virtualises the reading while the wait stays real buys nothing. That makes it the virtual-clock infrastructure #114 lists for this layer, not a two-line injection, and improvising it inside this ticket is how a small change becomes six review rounds.

The sequencing note here can be dropped, though: it said the seam "overlaps #102's area, so the two want sequencing together", and #102 is now closed as documented-and-declined.

---

Generated by Claude Code