Fix packet buffer read index after buffer resize

[zedkipp]

Description

In #712 I encountered a test that failed in two peculiar ways. After debugging, there seemed to be a bug in the packet buffer resize logic that would result in one of the failure modes (connection reads erroneously blocking). This PR specifically addresses the test failure mode where the test times out due to erroneously blocking on reading a packet.

The change fixes a bug that caused the packet circular buffer to have an incorrect read index after a buffer resize occurs when the read index is non-zero. When this occurred, reading from a connection would erroneously block (packet(s) are in the buffer) and some buffered packets could be lost.

Reference issue

#712

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.52%. Comparing base (806ff2f) to head (3b5d316).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #713      +/-   ##
==========================================
- Coverage   78.63%   78.52%   -0.11%     
==========================================
  Files         101      101              
  Lines        6828     6828              
==========================================
- Hits         5369     5362       -7     
- Misses       1085     1091       +6     
- Partials      374      375       +1     

Flag	Coverage Δ
go	78.55% <100.00%> (-0.11%)	⬇️
wasm	57.11% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Sean-Der]

Dang, that's some good debugging. Thank you @Zacho2

[Sean-Der]

Would you mind making the PR against pion/transport also @Zacho2 ?

[zedkipp]

@Sean-Der I would be happy to make that PR, I'm just not sure exactly what needs to change in pion/transport. Could you point me in the right direction?

I also re-pushed with a slight change to the read index fix (I realized both resize cases need to update the read index), and I simplified the test.

[Sean-Der]

I saw in buffer.go that this was copied from https://github.com/pion/transport/blob/master/packetio/buffer.go

Can you tell if it suffers from the same bug?

[zedkipp]

The same bug does not seem present to me. The Buffer.grow() method seems to reset head (read) pointer to zero just like my commit. https://github.com/pion/transport/blob/43c8901250838c2740c30a7d7442deba1978fe1d/packetio/buffer.go#L115

[Sean-Der]

Great news, thank for you for checking that!