Skip to content

Commit 364d054

Browse files
committed
Fix a bad race condition that causes intermittent socket receive
failures. The issue is caused by the code checking socket status *after* checking if bytes are available. Bytes may have become available between the last byte available check and the socket status check. This causes us to incorrectly ignore the newly available bytes and return 0 to the caller, which in any blocking/timeout scenario tells the caller no more bytes will ever be available. This was introduced in commit 89b9f10 See also adafruit#151 as a fix for adafruit#135
1 parent 9000b5a commit 364d054

File tree

1 file changed

+11
-2
lines changed

1 file changed

+11
-2
lines changed

adafruit_wiznet5k/adafruit_wiznet5k_socketpool.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,14 @@ def recv_into( # pylint: disable=unused-argument
601601
self._buffer = self._buffer[bytes_to_read:]
602602
# explicitly recheck num_to_read to avoid extra checks
603603
continue
604-
604+
# We need to read the socket status here before seeing if any bytes are available.
605+
# Why? Because otherwise we have a bad race condition in the if/elif/... logic below
606+
# that can cause recv_into to fail when the other side closes the connection after
607+
# sending bytes. The problem is that initially we can have num_avail=0 while the
608+
# socket state is not in CLOSED or CLOSED_WAIT. Then after the if but before the elif
609+
# that checks the socket state, bytes arrive and the other end closes the connection.
610+
# So now bytes are available but we see the CLOSED/CLOSED_WAIT state and ignore them.
611+
status_before_getting_available = self._status
605612
num_avail = self._available()
606613
if num_avail > 0:
607614
last_read_time = ticks_ms()
@@ -620,7 +627,9 @@ def recv_into( # pylint: disable=unused-argument
620627
elif num_read > 0:
621628
# We got a message, but there are no more bytes to read, so we can stop.
622629
break
623-
elif self._status in (
630+
# See note where we set status_before_getting_available for why we can't just check
631+
# _status here
632+
elif status_before_getting_available in (
624633
wiznet5k.adafruit_wiznet5k.SNSR_SOCK_CLOSED,
625634
wiznet5k.adafruit_wiznet5k.SNSR_SOCK_CLOSE_WAIT,
626635
):

0 commit comments

Comments
 (0)