fix(tcp): only accept connections that may send

[jounathaen]

This PR prevents returning sockets from accept() that are in the SYN-RECEIVED state and have thus not been ESTABLISHED yet and might never be. This is causing #906, but has also been observed in other scenarios.

When we listen for connections, the following state transitions take place:

      TCP A                                                TCP B

  1.  CLOSED                                               LISTEN

  2.  SYN-SENT    --> <SEQ=100><CTL=SYN>               --> SYN-RECEIVED

  3.  ESTABLISHED <-- <SEQ=300><ACK=101><CTL=SYN,ACK>  <-- SYN-RECEIVED

  4.  ESTABLISHED --> <SEQ=101><ACK=301><CTL=ACK>       --> ESTABLISHED

  5.  ESTABLISHED --> <SEQ=101><ACK=301><CTL=ACK><DATA> --> ESTABLISHED

          Basic 3-Way Handshake for Connection Synchronization

                                Figure 7.

See RFC 793 for details.

If, for whatever reason, we don't get SYN,ACK in 3., we should not return the socket to the application, since we might never get it and thus the connection has not been ESTABLISHED yet. Currently we do, since we currently use Socket::is_active to decide whether it is ready to be returned. That method returns true if the socket is not CLOSED, TIME-WAIT or LISTEN.

This PR avoids returning such sockets to the application by using Socket::may_send instead, which is true if the socket is ESTABLISHED or CLOSE-WAIT.

CLOSE-WAIT explicitly makes sense and can be produced when the other side is opening a connection and closes it immediately. Still, data can be received that was sent intermittently, and data can be sent, since it was ESTABLISHED before.

Closes #906.

[github-actions]

Benchmark Results

Details

Benchmark	Current: e82b11d	Previous: ce6a236	Performance Ratio
startup_benchmark Build Time	110.52 s	107.35 s	1.03 ❗
startup_benchmark File Size	0.75 MB	0.75 MB	1.00 ❗
Startup Time - 1 core	1.01 s (±0.05 s)	0.93 s (±0.07 s)	1.09
Startup Time - 2 cores	1.00 s (±0.03 s)	0.94 s (±0.05 s)	1.07 ❗
Startup Time - 4 cores	1.03 s (±0.04 s)	0.99 s (±0.04 s)	1.04
multithreaded_benchmark Build Time	118.67 s	110.67 s	1.07 ❗
multithreaded_benchmark File Size	0.87 MB	0.87 MB	1.00 ❗
Multithreaded Pi Efficiency - 2 Threads	94.33 % (±9.04 %)	92.42 % (±11.43 %)	1.02
Multithreaded Pi Efficiency - 4 Threads	46.31 % (±4.73 %)	45.92 % (±5.46 %)	1.01
Multithreaded Pi Efficiency - 8 Threads	25.79 % (±2.50 %)	25.34 % (±3.17 %)	1.02
micro_benchmarks Build Time	94.76 s	90.29 s	1.05 ❗
micro_benchmarks File Size	0.87 MB	0.87 MB	1.00 ❗
Scheduling time - 1 thread	73.07 ticks (±3.85 ticks)	66.89 ticks (±4.01 ticks)	1.09 ❗
Scheduling time - 2 threads	41.55 ticks (±4.34 ticks)	37.83 ticks (±4.60 ticks)	1.10
Micro - Time for syscall (getpid)	3.84 ticks (±0.18 ticks)	3.51 ticks (±0.24 ticks)	1.09 ❗
Memcpy speed - (built_in) block size 4096	73724.10 MByte/s (±50909.30 MByte/s)	81839.10 MByte/s (±56601.98 MByte/s)	0.90
Memcpy speed - (built_in) block size 1048576	29304.35 MByte/s (±24105.76 MByte/s)	30096.83 MByte/s (±24484.80 MByte/s)	0.97
Memcpy speed - (built_in) block size 16777216	24644.48 MByte/s (±20214.94 MByte/s)	26824.39 MByte/s (±22148.06 MByte/s)	0.92
Memset speed - (built_in) block size 4096	73988.63 MByte/s (±51084.31 MByte/s)	82211.20 MByte/s (±56852.70 MByte/s)	0.90
Memset speed - (built_in) block size 1048576	30056.42 MByte/s (±24530.63 MByte/s)	30820.35 MByte/s (±24892.88 MByte/s)	0.98
Memset speed - (built_in) block size 16777216	25396.88 MByte/s (±20716.80 MByte/s)	27568.62 MByte/s (±22596.81 MByte/s)	0.92
Memcpy speed - (rust) block size 4096	66968.75 MByte/s (±46697.82 MByte/s)	71055.92 MByte/s (±49922.07 MByte/s)	0.94
Memcpy speed - (rust) block size 1048576	29371.30 MByte/s (±24107.09 MByte/s)	30251.51 MByte/s (±24549.45 MByte/s)	0.97
Memcpy speed - (rust) block size 16777216	23813.19 MByte/s (±19650.09 MByte/s)	26489.85 MByte/s (±21826.15 MByte/s)	0.90
Memset speed - (rust) block size 4096	67189.44 MByte/s (±46860.85 MByte/s)	71343.28 MByte/s (±50124.43 MByte/s)	0.94
Memset speed - (rust) block size 1048576	30138.06 MByte/s (±24558.71 MByte/s)	31010.33 MByte/s (±24987.76 MByte/s)	0.97
Memset speed - (rust) block size 16777216	24506.33 MByte/s (±20104.42 MByte/s)	27257.69 MByte/s (±22311.78 MByte/s)	0.90
alloc_benchmarks Build Time	91.01 s	86.24 s	1.06 ❗
alloc_benchmarks File Size	0.83 MB	0.83 MB	1.00 ❗
Allocations - Allocation success	100.00 %	100.00 %	1
Allocations - Deallocation success	100.00 %	100.00 %	1
Allocations - Pre-fail Allocations	100.00 %	100.00 %	1
Allocations - Average Allocation time	4048.82 Ticks (±75.69 Ticks)	4014.47 Ticks (±73.89 Ticks)	1.01
Allocations - Average Allocation time (no fail)	4048.82 Ticks (±75.69 Ticks)	4014.47 Ticks (±73.89 Ticks)	1.01
Allocations - Average Deallocation time	643.38 Ticks (±29.04 Ticks)	671.96 Ticks (±52.84 Ticks)	0.96
mutex_benchmark Build Time	91.45 s	88.98 s	1.03 ❗
mutex_benchmark File Size	0.87 MB	0.87 MB	1.00 ❗
Mutex Stress Test Average Time per Iteration - 1 Threads	13.84 ns (±0.73 ns)	13.58 ns (±0.67 ns)	1.02
Mutex Stress Test Average Time per Iteration - 2 Threads	17.00 ns (±0.94 ns)	17.56 ns (±6.54 ns)	0.97

This comment was automatically generated by workflow using github-action-benchmark.

[mkroening]

@stlankes suggested using Socket::may_send, which exactly matches the functionality.

[mkroening]

One thing I am wondering about now is the symmetry with Socket::may_recv.

Should we check for the union of states (may_send() || may_recv()) or the intersection (state() == State::Established)? Or is there a specific reason we need only may_send?

[mkroening]

After some more testing around #906 as well as with a separate scenario, I believe that we should accept CLOSE-WAIT sockets. The other side might already have closed their send side, but the socket may still contain data to receive, and we can still answer.

[mkroening]

The issue this fixes is different from #2292, unfortunately.

[mkroening]

This does fix #906, though. 🥳

[mkroening]

I have adjusted the code according to @stlankes's suggestion and added a comment explaining the logic. I have also adjusted the PR description.

[mkroening]

This is great! Thanks so much for putting in the work for this! :)