Skip to content

Commit

Permalink
Remove yield usage from portal stream and protocol (status-im#1602)
Browse files Browse the repository at this point in the history
  • Loading branch information
kdeme authored Jun 15, 2023
1 parent d7f4051 commit dd4da74
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 38 deletions.
12 changes: 2 additions & 10 deletions fluffy/network/wire/portal_protocol.nim
Original file line number Diff line number Diff line change
Expand Up @@ -609,20 +609,12 @@ proc findContent*(p: PortalProtocol, dst: Node, contentKey: ByteList):
return err("Trying to connect to node with unknown address")

# uTP protocol uses BE for all values in the header, incl. connection id
let connFuture = p.stream.connectTo(
let connectionResult =
await p.stream.connectTo(
nodeAddress.unsafeGet(),
uint16.fromBytesBE(m.connectionId)
)

yield connFuture

var connectionResult: Result[UtpSocket[NodeAddress], string]

if connFuture.completed():
connectionResult = connFuture.read()
else:
raise connFuture.error

if connectionResult.isErr():
debug "uTP connection error while trying to find content",
error = connectionResult.error
Expand Down
38 changes: 10 additions & 28 deletions fluffy/network/wire/portal_stream.nim
Original file line number Diff line number Diff line change
Expand Up @@ -131,42 +131,24 @@ proc connectTo*(
nodeAddress: NodeAddress,
connectionId: uint16):
Future[Result[UtpSocket[NodeAddress], string]] {.async.} =
let connectFut = stream.transport.connectTo(nodeAddress, connectionId)

# using yield, not await, as await does not play nice with cancellation
# interacting with async procs which allocates some resource
yield connectFut

var socketRes: ConnectionResult[NodeAddress]

if connectFut.completed():
socketRes = connectFut.read()
else:
raise connectFut.error

if socketRes.isErr():
case socketRes.error.kind
let connectRes = await stream.transport.connectTo(nodeAddress, connectionId)
if connectRes.isErr():
case connectRes.error.kind
of SocketAlreadyExists:
# This means that there is already a socket to this nodeAddress with given
# connection id. It probably means that a peersent us a connection id
# which is already in use..
# For now just fail the connection and return an error. Another strategy
# to consider would be to check what is the connection status, and then
# re-use it, or close it and retry connection.
# connection id. This means that a peer sent us a connection id which is
# already in use. The connection is failed and an error returned.
let msg = "Socket to " & $nodeAddress & "with connection id: " &
$connectionId & " already exists"
return err(msg)
of ConnectionTimedOut:
# Another strategy for handling this error would be to retry connecting a
# few times before giving up. But we know (as we control the uTP impl)
# that this error will only occur when a SYN packet was re-sent 3 times
# and failed to be acked. This should be enough of indication that the
# remote host is not reachable.
# A time-out here means that a uTP SYN packet was re-sent 3 times and
# failed to be acked. This should be enough of indication that the
# remote host is not reachable and no new connections are attempted.
let msg = "uTP timeout while trying to connect to " & $nodeAddress
return err(msg)

let socket = socketRes.get()
return ok(socket)
else:
return ok(connectRes.get())

proc writeContentRequest(
socket: UtpSocket[NodeAddress], stream: PortalStream,
Expand Down

0 comments on commit dd4da74

Please sign in to comment.