The previous iface observer refactoring exposes some existing code
readability issues (as pointed out by @tatetian). Ideally, it is a bit
better to fix readability issues before doing new code refactoring, but
to avoid complex rebase conflicts, this commit fixes these issues only
after the code refactoring has landed.
The TCP logic consists of multiple files, each file representing a TCP
state. Since the UPP logic also consists of two states, `BoundDatagram`
and `UnboundDatagram`, this commit puts them into separate files, just
like TCP does.
There is no need to get the bound socket by cloning the first element
from `self.backlog_sockets`. If we want a bound socket, we can always
use `self.bound_socket` directly.
Finally, this commit implements an iface event observer trait for the
`ConnectingStream`, `ListenStream`, and `ConnectedStream` states in
`StreamSocket`, as well as the `BoundDatagram` state in
`DatagramSocket`. It also moves the `Pollee` from `AnyBoundSocket` to
these observer implementors.
What I have tried to do is minimize the semantic changes. Ideally, this
commit should be a pure refactor commit, meaning that even if the
sematics of the previous code is wrong, the sematics after this commit
should be wrong in the same way. Fixing the wrong sematics should be
done in a separate commit afterwards.
However, keeping exactly the same sematics for `ListenStream` is hard.
We used to maintain a `Pollee` for each `BacklogSocket`, but now we can
just maintain one `Pollee` for the whole `ListenStream`. However,
implementing the correct semantics looks much easier, so we just do it.
For `ConnectingStream`, it used to share the same `Pollee` logic with
`ConnectedStream` (because the `Pollee` was maintained by
`AnyBoundSocket`, which is used by both). Now we write the `Pollee`
logic separately for `ConnectingStream`, so we can just write a correct
one or try to reuse the logic in `ConnectingStream`. This commit does
the former.
There should be no semantic changes for `ConnectedStream` in
`StreamSocket` and `BoundDatagram` in `DatagramSocket`.
For TCP streams we used to have three states, e.g. `InitStream`,
`ListenStream`, `ConnectedStream`. If the socket is not bound, it is in
the `InitStream` state. If the socket is bound, it is still in that
state. Most seriously, if the socket is connecting to the remote peer,
but the connection has not been established, it is also in the
`InitStream` state.
While the socket is trying to connect to its peer, it needs to handle
iface events to update its internal state. So it is expected to
implement the trait that observes such events for this later. However,
the reality that sockets connecting to their peers are mixed in with
other unbound and bound sockets in the `InitStream` will complicate
things.
In fact, the connecting socket should belong to an independent state. It
does not share too much logic with unbound and bound sockets in the
`InitStream`. So in this commit we will decouple that and create a new
`ConnectingStream` state.
Changes in this commit were suggested by @StevenJiang1110. HUP/RDHUP
events are not correctly updated for TCP sockets (#529), so we are
removing this to avoid further confusion.
So far it is quite common to put the `AnyUnboundSocket` in an enum
variant, e.g. the following code snippet.
```rust
enum Inner {
Unbound(AlwaysSome<Box<AnyUnboundSocket>>),
Bound(AlwaysSome<Arc<AnyBoundSocket>>),
// ...
}
```
However, this pattern is very memory inefficient because the size
difference between two enum variants is significant. The size of
`AnyUnboundSocket` is much larger than the size of
`Arc<AnyBoundSocket>`, where the latter is simply a pointer and a
reference counter.
In fact, we're about to trigger Clippy's large_enum_variant warning.
We're just below its threshold, so the warning doesn't appear.
The solution is simple: If we put `AnyBoundSocket` in `Arc`, we should
also put `AnyUnboundSocket` in `Box`. This way the sizes of different
enum variants will be similar.
For TCP streams we have packed their different states with `Arc`, e.g.
`InitStream`, `ConnectedStream`, and `ListenStream`. Later we will
implement an trait that observes iface events for some of these states.
For UDP datagrams, they can be in the unbound state and the bound state.
If we want to implement the observer trait for the bound state, we need
to wrap it with `Arc` so that it can be registered with
`AnyBoundSocket`.
Alternatively, we can implement the observer trait directly on the UDP
datagram structure (i.e. `DatagramSocket`). However, there are literally
no events to handle if the socket is not bound at all (i.e. it is in the
unbound state). So a more efficient way is to implement the observer
trait only for the bound state, which motivates changes in this commit.
We used to use `is_nonblocking()` in TCP streams, but use
`nonblocking()` in UDP datagrams.
Since `is_nonblocking()` is generally preferred, this commit renames
`nonblocking()` to `is_nonblocking()` in UDP datagrams.
Jianfeng Jiang
Ruihan Li
Chen Chengjun
Hsy-Intel
Chuandong Li
LI Qing
Yuke Peng
Zhang Junyang