Skip to content

Commit

Permalink
fix(ConnectionManager): it maintains the Peer.ConnectedAdddress
Browse files Browse the repository at this point in the history
  • Loading branch information
richardschneider committed May 2, 2019
1 parent a53167c commit 5fd863e
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 20 deletions.
13 changes: 10 additions & 3 deletions src/ConnectionManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Threading;

namespace PeerTalk
{
Expand Down Expand Up @@ -37,11 +38,11 @@ public class ConnectionManager
public event EventHandler<MultiHash> PeerDisconnected;

/// <summary>
/// Gets the current connections.
/// Gets the current active connections.
/// </summary>
public IEnumerable<PeerConnection> Connections => connections.Values
.SelectMany(c => c)
.Where(c => c.Stream != null && c.Stream.CanRead && c.Stream.CanWrite);
.Where(c => c.IsActive);

/// <summary>
/// Determines if a connection exists to the specified peer.
Expand Down Expand Up @@ -83,7 +84,7 @@ public bool TryGet(Peer peer, out PeerConnection connection)
}

connection = conns
.Where(c => c.Stream != null && c.Stream.CanRead && c.Stream.CanWrite)
.Where(c => c.IsActive)
.FirstOrDefault();

return connection != null;
Expand Down Expand Up @@ -125,6 +126,10 @@ public PeerConnection Add(PeerConnection connection)
}
);

if (connection.RemotePeer.ConnectedAddress == null)
{
connection.RemotePeer.ConnectedAddress = connection.RemoteAddress;
}
connection.Closed += (s, e) => Remove(e);
return connection;
}
Expand Down Expand Up @@ -172,6 +177,7 @@ public bool Remove(PeerConnection connection)
}
else
{
connection.RemotePeer.ConnectedAddress = null;
PeerDisconnected?.Invoke(this, connection.RemotePeer.Id);
}
return true;
Expand All @@ -194,6 +200,7 @@ public bool Remove(MultiHash id)
}
foreach (var conn in conns)
{
conn.RemotePeer.ConnectedAddress = null;
conn.Dispose();
}

Expand Down
15 changes: 11 additions & 4 deletions src/PeerConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,17 @@ public class PeerConnection : IDisposable
/// </value>
public bool IsIncoming { get; set; }

/// <summary>
/// Determines if the connection to the remote can be used.
/// </summary>
/// <value>
/// <b>true</b> if the connection is active.
/// </value>
public bool IsActive
{
get { return Stream != null && Stream.CanRead && Stream.CanWrite; }
}

/// <summary>
/// The duplex stream between the two peers.
/// </summary>
Expand Down Expand Up @@ -355,10 +366,6 @@ protected virtual void Dispose(bool disposing)
statsStream = null;
}
}
if (RemotePeer != null && RemotePeer.ConnectedAddress == RemoteAddress)
{
RemotePeer.ConnectedAddress = null;
}
SecurityEstablished.TrySetCanceled();
IdentityEstablished.TrySetCanceled();
IdentityEstablished.TrySetCanceled();
Expand Down
5 changes: 2 additions & 3 deletions src/Swarm.cs
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,8 @@ async Task<PeerConnection> Dial(Peer remote, IEnumerable<MultiAddress> addrs, Ca
}
catch (Exception e)
{
var attemped = string.Join(", ", possibleAddresses.Select(a => a.ToString()));
log.Warn($"Cannot dial {attemped}", e);
throw new Exception($"Cannot dial {remote}.", e);
}

Expand Down Expand Up @@ -593,7 +595,6 @@ async Task<PeerConnection> DialAsync(Peer remote, MultiAddress addr, Cancellatio
}

// Build the connection.
remote.ConnectedAddress = addr;
var connection = new PeerConnection
{
IsIncoming = false,
Expand Down Expand Up @@ -814,8 +815,6 @@ async void OnRemoteConnect(Stream stream, MultiAddress local, MultiAddress remot

connection.RemotePeer = RegisterPeer(connection.RemotePeer);
connection.RemoteAddress = new MultiAddress($"{remote}/ipfs/{connection.RemotePeer.Id}");
connection.RemotePeer.ConnectedAddress = connection.RemoteAddress;

var actual = Manager.Add(connection);
if (actual == connection)
{
Expand Down
28 changes: 18 additions & 10 deletions test/ConnectionManagerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,34 +113,42 @@ public void Add_Duplicate_PeerConnectedAddress()
}

[TestMethod]
public void Remove_Duplicate_PeerConnectedAddress()
public void Maintains_PeerConnectedAddress()
{
var address = "/ip6/::1/tcp/4007";
var address1 = "/ip4/127.0.0.1/tcp/4007";
var address2 = "/ip4/127.0.0.2/tcp/4007";

var manager = new ConnectionManager();
var peer = new Peer { Id = aId, ConnectedAddress = address };
var a = new PeerConnection { RemotePeer = peer, RemoteAddress = address, Stream = Stream.Null };
var b = new PeerConnection { RemotePeer = peer, RemoteAddress = address, Stream = Stream.Null };
var peer = new Peer { Id = aId };
var a = new PeerConnection { RemotePeer = peer, RemoteAddress = address1, Stream = Stream.Null };
var b = new PeerConnection { RemotePeer = peer, RemoteAddress = address2, Stream = Stream.Null };

Assert.AreSame(a, manager.Add(a));
Assert.IsTrue(manager.IsConnected(peer));
Assert.AreEqual(1, manager.Connections.Count());
Assert.IsNotNull(a.Stream);
Assert.AreEqual(address, peer.ConnectedAddress);
Assert.AreEqual(address1, peer.ConnectedAddress);

Assert.AreSame(b, manager.Add(b));
Assert.IsTrue(manager.IsConnected(peer));
Assert.AreEqual(2, manager.Connections.Count());
Assert.IsNotNull(a.Stream);
Assert.IsNotNull(b.Stream);
Assert.AreEqual(address, peer.ConnectedAddress);
Assert.AreEqual(address1, peer.ConnectedAddress);

Assert.IsTrue(manager.Remove(b));
Assert.IsTrue(manager.Remove(a));
Assert.IsTrue(manager.IsConnected(peer));
Assert.AreEqual(1, manager.Connections.Count());
Assert.IsNotNull(a.Stream);
Assert.IsNull(a.Stream);
Assert.IsNotNull(b.Stream);
Assert.AreEqual(address2, peer.ConnectedAddress);

Assert.IsTrue(manager.Remove(b));
Assert.IsFalse(manager.IsConnected(peer));
Assert.AreEqual(0, manager.Connections.Count());
Assert.IsNull(a.Stream);
Assert.IsNull(b.Stream);
Assert.AreEqual(address, peer.ConnectedAddress);
Assert.IsNull(peer.ConnectedAddress);
}

[TestMethod]
Expand Down
3 changes: 3 additions & 0 deletions test/PeerConnectionTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,17 @@ public void Disposing()
{
++closeCount;
};
Assert.IsTrue(connection.IsActive);
Assert.IsNotNull(connection.Stream);

connection.Dispose();
Assert.IsFalse(connection.IsActive);
Assert.IsNull(connection.Stream);

// Can be disposed multiple times.
connection.Dispose();

Assert.IsFalse(connection.IsActive);
Assert.AreEqual(1, closeCount);
}

Expand Down

0 comments on commit 5fd863e

Please sign in to comment.