FactbirdHQ / ublox-short-range-rs

A driver crate for the entire u-blox short range family in Rust
6 stars 2 forks source link

Socket handle tied to peer handle causes issues #30

Closed unizippro closed 2 years ago

unizippro commented 2 years ago

The libreary uses peer handles as socket handles, this causes major issues. The module reuses peer handles imediatly after closing them, often only twice before incrementing for some reason. Sockets are always closed by reciving a EDM_URC disconnect event. This causes all closes to be through ShutdownForWrite. This means even if a socket is closed, it will excist in the set untill the timeout is reached(15 sek). If a new socket is created and connected, you will have 2 with the same handle. This causes a close to only change the first(old) one, leaving the second(new) one in the state Connected or WaitingForConnection in the socketset. This also causes crossover during connection, as the state of the old socket is read, and connect is exited before an EDM_URC has arrived

An example is shown below:

110889 DEBUG [TCP] Opening socket
110890 DEBUG Adding socket! 0 Tcp
110890 DEBUG [Socket Set] Adding to: [(Handle(2), Tcp(State::ShutdownForWrite))]
110890 DEBUG [TCP] Connect socket
110909 DEBUG Sending command with too long payload (81 bytes) to log! #(Connection attempt)
110939 DEBUG [Handle(0)] TCP state change: State::Created -> State::WaitingForConnect
110939 DEBUG [Handle(0)] Updating handle Handle(2)
110939 TRACE [TCP] Connecting socket: Handle(2) to url: tcp://54.194.121.54:8883/?ca=root_ca&cert=cert&privKey=priv_key
110939 DEBUG Network connected!
110939 INFO  MQTT connecting..
110942 ERROR [send] NetworkError::Write
110942 DEBUG Disconnecting!
110942 DEBUG [TCP] Closing socket: Handle(2)
110942 DEBUG Removing socket! 2 Some(Tcp)
110959 DEBUG Sending command with too long payload (67 bytes) to log! #(DNS)
110980 TRACE [EDM_URC] IPv4ConnectEvent! Channel_id: ChannelId(2)
110980 TRACE [EDM_MAP] Handle(2) tied to ChannelId(2)
110980 DEBUG [Handle(2)] TCP state change: State::WaitingForConnect -> State::Connected
110980 TRACE [URC] PeerConnected
111021 TRACE [URC] PingResponse
111021 DEBUG [TCP] Opening socket
111022 DEBUG Adding socket! 0 Tcp
111022 DEBUG [Socket Set] Adding to: [(Handle(2), Tcp(State::Connected))]

As it requires one empty space in the set, it can never lock the set, as it will leave one open space.

This is recreatable by creating a connection to anywhere, closing it, and creating a connection to the same place again.

Solution

Decouble Peer handles and socket handles, in the same way peer handle and channel id is set up.

Alternative

Close any sockets with the same peer handle, as they do not match the modem state anymore. This is quite an easy fix.

MathiasKoch commented 2 years ago

Closed by #38