Skip to content

Commit

Permalink
Fixed android connect to non listening peer bug app crash bug. Change…
Browse files Browse the repository at this point in the history 
…d what IPAddress.Any does when calling startlistening using override that takes a connection type. Added validation checks to packetbuilder.
  • Loading branch information
MarcF committed Mar 27, 2014
1 parent 5a414e8 commit 08e8e26
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 42 deletions.
15 changes: 13 additions & 2 deletions DebugTests/DebugTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,19 @@ static class DebugTest
public static void RunExample()
{
//Get the serializer and data processors
TCPConnectionListener listener = new TCPConnectionListener(NetworkComms.DefaultSendReceiveOptions, ApplicationLayerProtocolStatus.Enabled);
Connection.StartListening(listener, new IPEndPoint(IPAddress.Any, 10000));
//TCPConnectionListener listener = new TCPConnectionListener(NetworkComms.DefaultSendReceiveOptions, ApplicationLayerProtocolStatus.Enabled);
Connection.StartListening(ConnectionType.TCP, new IPEndPoint(IPAddress.Any, 10000));

Console.WriteLine("Listening on:");
foreach (var endpoint in Connection.ExistingLocalListenEndPoints(ConnectionType.TCP))
Console.WriteLine("\t-> {0}", endpoint.ToString());

NetworkComms.AppendGlobalIncomingPacketHandler<string>("Data", (header, connection, message) =>
{
Console.WriteLine("Connection with {0} saying {1}", connection, message);
});

TCPConnection.GetConnection(new ConnectionInfo("192.168.0.117", 10000)).SendObject("Data", "hello");

Console.WriteLine("Client done!");
Console.ReadKey();
Expand Down
15 changes: 7 additions & 8 deletions NetworkCommsDotNet/Connection/ConnectionListeners.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,8 @@ public static List<ConnectionListenerBase> StartListening<T>(ConnectionType conn
foreach(IPAddress address in HostInfo.IP.FilteredLocalAddresses())
localListenIPEndPoints.Add(new IPEndPoint(address, desiredLocalIPEndPoint.Port));

//We still include the IPAddress.Any address as some platforms
//require this to receive broadcasts
localListenIPEndPoints.Add(desiredLocalIPEndPoint);
//We could also listen on the IPAddress.Any adaptor here
//but it is not supported by all platforms so use the specific start listening override instead
}
else
localListenIPEndPoints.Add(desiredLocalIPEndPoint);
Expand Down Expand Up @@ -151,8 +150,8 @@ public static List<ConnectionListenerBase> StartListening<T>(ConnectionType conn
/// when method returns to determine the <see cref="IPEndPoint"/> used.
/// </summary>
/// <param name="listener">The listener to use.</param>
/// <param name="desiredLocalEndPoint">The desired local <see cref="IPEndPoint"/> to use for listening. Use a port number
/// of 0 to dynamically select a port.</param>
/// <param name="desiredLocalEndPoint">The desired local <see cref="IPEndPoint"/> to use for listening. IPAddress.Any corresponds with listening on
/// 0.0.0.0. Use a port number of 0 to dynamically select a port.</param>
/// <param name="useRandomPortFailOver">If true and the requested local port is not available will select one at random.
/// If false and provided port is unavailable will throw <see cref="CommsSetupShutdownException"/></param>
public static void StartListening<T>(ConnectionListenerBase listener, T desiredLocalEndPoint, bool useRandomPortFailOver = false) where T : EndPoint
Expand Down Expand Up @@ -214,8 +213,8 @@ public static void StartListening<T>(ConnectionListenerBase listener, T desiredL
/// when method returns to determine the <see cref="IPEndPoint"/>s used.
/// </summary>
/// <param name="listeners">The listeners to use.</param>
/// <param name="desiredLocalEndPoints">The desired local <see cref="IPEndPoint"/>s to use for listening. Use a port number
/// of 0 to dynamically select a port.</param>
/// <param name="desiredLocalEndPoints">The desired local <see cref="IPEndPoint"/>s to use for listening. IPAddress.Any corresponds with listening on
/// 0.0.0.0. Use a port number of 0 to dynamically select a port.</param>
/// <param name="useRandomPortFailOver">If true and the requested local port is not available will select one at random.
/// If false and provided port is unavailable will throw <see cref="CommsSetupShutdownException"/></param>
public static void StartListening<T>(List<ConnectionListenerBase> listeners, List<T> desiredLocalEndPoints, bool useRandomPortFailOver) where T : EndPoint
Expand Down Expand Up @@ -394,7 +393,7 @@ public static List<EndPoint> ExistingLocalListenEndPoints(ConnectionType connect
return ExistingLocalListenEndPoints(connectionType, new BluetoothEndPoint(BluetoothAddress.None, BluetoothService.Empty));
#endif
else
throw new ArgumentException("Method ExistingLocalListenEndPoints is not defined for the conneciton type: " + connectionType.ToString(), "connectionType");
throw new ArgumentException("Method ExistingLocalListenEndPoints is not defined for the connection type: " + connectionType.ToString(), "connectionType");
}

/// <summary>
Expand Down
23 changes: 15 additions & 8 deletions NetworkCommsDotNet/Connection/TCP/TCPConnection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,13 +209,10 @@ private void ConnectSocket()
WaitHandle connectionWait = ar.AsyncWaitHandle;
try
{
if (!ar.AsyncWaitHandle.WaitOne(NetworkComms.ConnectionEstablishTimeoutMS, false))
{
tcpClient.Close();
if (!connectionWait.WaitOne(NetworkComms.ConnectionEstablishTimeoutMS, false))
connectSuccess = false;
}

tcpClient.EndConnect(ar);
else
tcpClient.EndConnect(ar);
}
finally
{
Expand Down Expand Up @@ -692,8 +689,15 @@ protected override void CloseConnectionSpecific(bool closeDueToError, int logLoc
//Try to close the tcpClient
try
{
tcpClient.Client.Disconnect(false);
tcpClient.Client.Close();
if (tcpClient.Client!=null)
{
tcpClient.Client.Disconnect(false);

#if !ANDROID
//Throws uncatchable exception in android
tcpClient.Client.Close();
#endif
}
}
catch (Exception)
{
Expand All @@ -702,7 +706,10 @@ protected override void CloseConnectionSpecific(bool closeDueToError, int logLoc
//Try to close the tcpClient
try
{
#if !ANDROID
//Throws uncatchable exception in android
tcpClient.Close();
#endif
}
catch (Exception)
{
Expand Down
42 changes: 28 additions & 14 deletions NetworkCommsDotNet/Tools/PacketBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,24 +122,33 @@ public void ClearNTopBytes(int numBytesToRemove)

//Reset the totalBytesRead
totalBytesCached -= bytesRemoved;

//Get rid of any null packets
List<byte[]> newPackets = new List<byte[]>(packets.Count);
for (int i = 0; i < packets.Count; i++)

if (totalBytesCached > 0)
{
if (packets[i] != null)
newPackets.Add(packets[i]);
}
packets = newPackets;
//Get rid of any null packets
List<byte[]> newPackets = new List<byte[]>(packets.Count);
for (int i = 0; i < packets.Count; i++)
{
if (packets[i] != null)
newPackets.Add(packets[i]);
}
packets = newPackets;

//Remove any -1 entries
List<int> newPacketActualBytes = new List<int>(packetActualBytes.Count);
for (int i = 0; i < packetActualBytes.Count; i++)
//Remove any -1 entries
List<int> newPacketActualBytes = new List<int>(packetActualBytes.Count);
for (int i = 0; i < packetActualBytes.Count; i++)
{
if (packetActualBytes[i] > -1)
newPacketActualBytes.Add(packetActualBytes[i]);
}
packetActualBytes = newPacketActualBytes;
}
else
{
if (packetActualBytes[i] > -1)
newPacketActualBytes.Add(packetActualBytes[i]);
//This is faster if we have removed everything
packets = new List<byte[]>();
packetActualBytes = new List<int>();
}
packetActualBytes = newPacketActualBytes;

//This is a really bad place to put a garbage collection as it hammers the CPU
//GC.Collect();
Expand All @@ -156,6 +165,11 @@ public void ClearNTopBytes(int numBytesToRemove)
/// <param name="partialPacket">A buffer which may or may not be full with valid bytes</param>
public void AddPartialPacket(int packetBytes, byte[] partialPacket)
{
if (packetBytes > partialPacket.Length)
throw new ArgumentException("packetBytes cannot be greater than the length of the provided partialPacket data.");
if (packetBytes < 0)
throw new ArgumentException("packetBytes cannot be negative.");

lock (Locker)
{
totalBytesCached += packetBytes;
Expand Down
41 changes: 31 additions & 10 deletions NetworkCommsDotNet/Tools/PeerDiscovery.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,14 @@ public static void EnableDiscoverable(DiscoveryMethod discoveryMethod)
if (discoveryMethod == DiscoveryMethod.UDPBroadcast)
#endif
{

#if iOS || ANDROID
//iOS and Android must include IPAddress.Any in the listening addresses to successfully receive broadcasts
List<IPAddress> localAddresses = new List<IPAddress>() { IPAddress.Any };
#else
//We should select one of the target points across all adaptors, no need for all adaptors to have
//selected a single uniform port which is what happens if we just pass IPAddress.Any to the StartListening method
List<IPAddress> localAddresses = HostInfo.IP.FilteredLocalAddresses();

#if iOS || ANDROID
//iOS and Android must also listen on IPAddress.Any to successfully receive UDP broadcasts
if (discoveryMethod == DiscoveryMethod.UDPBroadcast)
localAddresses.Add(IPAddress.Any);
#endif

List<ConnectionListenerBase> listeners = new List<ConnectionListenerBase>();
Expand All @@ -308,10 +308,16 @@ public static void EnableDiscoverable(DiscoveryMethod discoveryMethod)
{
try
{
List<ConnectionListenerBase> newlisteners = Connection.StartListening(discoveryMethod == DiscoveryMethod.UDPBroadcast ? ConnectionType.UDP : ConnectionType.TCP, new IPEndPoint(address, tryPort), true);
ConnectionListenerBase listener;
if (discoveryMethod == DiscoveryMethod.UDPBroadcast)
listener = new UDPConnectionListener(NetworkComms.DefaultSendReceiveOptions, ApplicationLayerProtocolStatus.Enabled, UDPOptions.None, true);
else
listener = new TCPConnectionListener(NetworkComms.DefaultSendReceiveOptions, ApplicationLayerProtocolStatus.Enabled, true);

Connection.StartListening(listener, new IPEndPoint(address, tryPort));

//Once we are successfully listening we can break
listeners.AddRange(newlisteners);
listeners.Add(listener);
break;
}
catch (Exception) { }
Expand Down Expand Up @@ -384,10 +390,16 @@ public static void EnableDiscoverable(DiscoveryMethod discoveryMethod, EndPoint
{
try
{
List<ConnectionListenerBase> newlisteners = Connection.StartListening(discoveryMethod == DiscoveryMethod.UDPBroadcast ? ConnectionType.UDP : ConnectionType.TCP, new IPEndPoint(address, tryPort), true);
ConnectionListenerBase listener;
if (discoveryMethod == DiscoveryMethod.UDPBroadcast)
listener = new UDPConnectionListener(NetworkComms.DefaultSendReceiveOptions, ApplicationLayerProtocolStatus.Enabled, UDPOptions.None, true);
else
listener = new TCPConnectionListener(NetworkComms.DefaultSendReceiveOptions, ApplicationLayerProtocolStatus.Enabled, true);

Connection.StartListening(listener, new IPEndPoint(address, tryPort));

//Once we are successfully listening we can break
_discoveryListeners.Add(discoveryMethod, newlisteners);
_discoveryListeners.Add(discoveryMethod, new List<ConnectionListenerBase>() { listener });
break;
}
catch (Exception) { }
Expand All @@ -399,7 +411,16 @@ public static void EnableDiscoverable(DiscoveryMethod discoveryMethod, EndPoint
else
{
//Based on the connection type select all local endPoints and then enable discoverable
_discoveryListeners.Add(discoveryMethod, Connection.StartListening(discoveryMethod == DiscoveryMethod.UDPBroadcast ? ConnectionType.UDP : ConnectionType.TCP, localDiscoveryEndPoint, true));
ConnectionListenerBase listener;
if (discoveryMethod == DiscoveryMethod.UDPBroadcast)
listener = new UDPConnectionListener(NetworkComms.DefaultSendReceiveOptions, ApplicationLayerProtocolStatus.Enabled, UDPOptions.None, true);
else
listener = new TCPConnectionListener(NetworkComms.DefaultSendReceiveOptions, ApplicationLayerProtocolStatus.Enabled, true);

Connection.StartListening(listener, localDiscoveryEndPoint);

//Once we are successfully listening we can break
_discoveryListeners.Add(discoveryMethod, new List<ConnectionListenerBase>() { listener });
}

//Add the packet handlers if required
Expand Down

0 comments on commit 08e8e26

Please sign in to comment.