Subject: [quicwg/base-drafts] assertion in GetPtoTimeAndSpace of recovery draft (#3910)

From: Kazu Yamamoto <[email-masked]>
Date: Wed, 15 Jul 2020 00:51:01 -0700

`assertion` in `GetPtoTimeAndSpace` fails in many times.

`GetPtoTimeAndSpace` is as follows:

```
GetPtoTimeAndSpace():
  duration = (smoothed_rtt + max(4 * rttvar, kGranularity))
      * (2 ^ pto_count)
  // Arm PTO from now when there are no inflight packets.
  if (no in-flight packets):
    assert(!PeerCompletedAddressValidation())
    if (has handshake keys):
      return (now() + duration), Handshake
    else:
      return (now() + duration), Initial
  pto_timeout = infinite
  pto_space = Initial
  ...
```

It is called from two places.

In the first place, `GetPtoTimeAndSpace()` is called when

- ack-eliciting packets in flight (roughly, `bytes_in_flight <= 0`)
- *OR* `!PeerCompletedAddressValidation()`

```
SetLossDetectionTimer():
  ...
  if (no ack-eliciting packets in flight &&
      PeerCompletedAddressValidation()):
    // There is nothing to detect lost, so no timer is set.
    // However, the client needs to arm the timer if the
    // server might be blocked by the anti-amplification limit.
    loss_detection_timer.cancel()
    return

  // Determine which PN space to arm PTO for.
  timeout, _ = GetPtoTimeAndSpace()
```

Int the second place, `GetPtoTimeAndSpace()` is called when

- `bytes_in_flight > 0`

```
OnLossDetectionTimeout():
  if (bytes_in_flight > 0):
    // PTO. Send new data if available, else retransmit old data.
    // If neither is available, send a single PING frame.
    _, pn_space = GetPtoTimeAndSpace()
    SendOneOrTwoAckElicitingPackets(pn_space)
```

So, it is not guaranteed that the following stands always:

- `bytes_in_flight <= 0`
- *AND* `!PeerCompletedAddressValidation()`

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3910

---

From: ianswett <[email-masked]>
Date: Thu, 16 Jul 2020 08:25:38 -0700

I read this a few times and clearly I'm missing something.  If bytes_in_flight > 0, then there must be no inflight packets.  The other call is preceded by:

So the only way we could enter this code AND have no packets in flight is if PeerCompletedAddressValidation() is false.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3910#issuecomment-659485799

---

From: Kazu Yamamoto <[email-masked]>
Date: Thu, 16 Jul 2020 18:40:07 -0700

@ianswett In short, you and pseudocode is correct.

I made two mistakes. One is explanation above (already fixed). The other is my implementation where the condition was reversed. I confirmed that fixed implementation does not generate assertion warnings.

Sorry for bothering you. Let's close this.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3910#issuecomment-659778696

---

From: Kazu Yamamoto <[email-masked]>
Date: Thu, 16 Jul 2020 18:40:08 -0700

Closed #3910.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/issues/3910#event-3555868211