Need Confirmation

[OneFingerCodingWarrior]

Hi: I changed the code a little bit for the following two reasones

1、manager.Peers.ActivePeers.Add(peer); positon is a little misleading, because there are lot of exception throwed from CheckEncryptionAsync or EndCheckEncryption

2、if handshake is finished successfully ,shoudle't we begin send requestes immediatlly?

internal async Task ConnectToPeer(TorrentManager manager, Peer peer)
        {
            // Connect to the peer.
            IConnection connection = ConnectionFactory.Create(peer.ConnectionUri);

            if (connection == null)
                return;

            var state = new AsyncConnectState(manager, connection, Stopwatch.StartNew ());
            PendingConnects.Add(state);
            manager.Peers.ConnectingToPeers.Add(peer);

            bool succeeded;
            String strError = String.Empty;

            try
            {
                await NetworkIO.ConnectAsync(connection);
                succeeded = true;
            }
            catch(Exception ex)
            {
                strError = ex.Message;
                succeeded = false;
            }

            PendingConnects.Remove (state);

//Actively Release Resources and Reduce GC Pressure
            state.Manager = null;
            state.Connection = null;
            state.Timer = null;

            if (manager.Engine == null ||
                !manager.Mode.CanAcceptConnections) {
                connection.Dispose ();
                return;
            }

            try
            {
                manager.Peers.ConnectingToPeers.Remove(peer);

                if (!succeeded)
                {
                    Logger.Log(null, "ConnectionManager - Failed to connect " + peer + " " + strError);

                    manager.RaiseConnectionAttemptFailed(
                        new PeerConnectionFailedEventArgs(manager, peer, Direction.Outgoing, "EndCreateConnection"));

                    peer.FailedConnectionAttempts++;
                    connection.Dispose();
                    manager.Peers.BusyPeers.Add(peer);
                }
                else
                {
                    PeerId id = new PeerId(peer, manager);
                    id.Connection = connection;

                    Logger.Log(id.Connection, " ConnectionManager - Connection opened");

                    await ProcessFreshConnection(id);
                }
            }
            catch (Exception)
            {
                // FIXME: Do nothing now?
                Logger.Log(null, " ConnectionManager check error " + peer + " " + strError);
            }
            finally
            {
                // Try to connect to another peer
                //TryConnect();//Is't this redundant ?
            }
        }

        internal async Task ProcessFreshConnection(PeerId id)
        {
            // If we have too many open connections, close the connection
            if (OpenConnections > this.MaxOpenConnections)
            {
                CleanupSocket (id, "Too many connections");
                return;
            }

            try
            {
                id.ProcessingQueue = true;
                // Increase the count of the "open" connections
                var initialData = await EncryptorFactory.CheckEncryptionAsync (id, 0, new[] { id.TorrentManager.InfoHash });
                await EndCheckEncryption(id, initialData);

                id.TorrentManager.Peers.ActivePeers.Add(id.Peer);

                id.TorrentManager.Peers.ConnectedPeers.Add(id);
                id.WhenConnected.Restart ();
                // Baseline the time the last block was received
                id.LastBlockReceived.Restart ();
            }
            catch (Exception ex)
            {
                Logger.Log(id.Connection, ex.Message);

                id.TorrentManager.RaiseConnectionAttemptFailed(
                    new PeerConnectionFailedEventArgs(id.TorrentManager, id.Peer, Direction.Outgoing, "ProcessFreshConnection: failed to encrypt"));

                //id.Connection.Dispose();
                //id.Connection = null;
                CleanupSocket(id, "ProcessFreshConnection Error " + ex.Message);
            }
        }

        private async Task EndCheckEncryption(PeerId id, byte[] initialData)
        {
            try
            {
                if (initialData != null && initialData.Length > 0)
                    throw new EncryptionException("unhandled initial data");

                EncryptionTypes e = engine.Settings.AllowedEncryption;
                if (id.Encryptor is RC4 && !Toolbox.HasEncryption(e, EncryptionTypes.RC4Full) ||
                    id.Encryptor is RC4Header && !Toolbox.HasEncryption(e, EncryptionTypes.RC4Header) ||
                    id.Encryptor is PlainTextEncryption && !Toolbox.HasEncryption(e, EncryptionTypes.PlainText))
                {
                    CleanupSocket(id, id.Encryptor.GetType().Name + " encryption is not enabled");
                }
                else
                {
                    // Create a handshake message to send to the peer
                    var handshake = new HandshakeMessage(id.TorrentManager.InfoHash, engine.PeerId, VersionInfo.ProtocolStringV100);
                    await PeerIO.SendMessageAsync (id.Connection, id.Encryptor, handshake, id.TorrentManager.UploadLimiter, id.Monitor, id.TorrentManager.Monitor);

                    // Receive their handshake
                    handshake = await PeerIO.ReceiveHandshakeAsync (id.Connection, id.Decryptor);
                    handshake.Handle(id);

                    id.TorrentManager.HandlePeerConnected(id, Direction.Outgoing);

                    // If there are any pending messages, send them otherwise set the queue processing as finished.
                    if (id.QueueLength > 0)
                        await ProcessQueue(id);
                    else
                        id.ProcessingQueue = false;

                    ReceiveMessagesAsync(id.Connection, id.Decryptor, id.TorrentManager.DownloadLimiter, id.Monitor, id.TorrentManager, id);
                    // Alert the engine that there is a new usable connection
                    //id.TorrentManager.HandlePeerConnected(id, Direction.Outgoing);
                }
            }
            catch(Exception ex)
            {
                Logger.Log(id.Connection, ex.Message);

                id.Peer.Encryption &= ~EncryptionTypes.RC4Full;
                id.Peer.Encryption &= ~EncryptionTypes.RC4Header;
                //CleanupSocket(id, "Failed encryptor check");
                throw;
            }
        }

        async Task ReceiveMessagesAsync (IConnection connection, IEncryption decryptor, RateLimiterGroup downloadLimiter, ConnectionMonitor monitor, TorrentManager torrentManager, PeerId id)
        {
            try
            {
                while (true)
                {
                    var message = await PeerIO.ReceiveMessageAsync(connection, decryptor, downloadLimiter, monitor, torrentManager);
                    if (id.Disposed)
                    {
                        if (message is PieceMessage msg)
                            ClientEngine.BufferManager.FreeBuffer(msg.Data);
                    }
                    else
                    {
                        id.LastMessageReceived.Restart();

                        if (PeerMessageTransferred != null)
                            RaisePeerMessageTransferred(new PeerMessageEventArgs(id.TorrentManager, message, Direction.Incoming, id));

                        message.Handle(id);
                    }
                }
            }
            catch(Exception ex)
            {
                Logger.Log(connection, ex.Message);
                CleanupSocket(id, "Could not receive a message");
            }
        }

        #endregion

        #region Methods

        internal void CleanupSocket(PeerId id, string message = null)
        {
            if (id == null || id.Disposed) // Sometimes onEncryptoError will fire with a null id
                return;

            try
            {
                // We can reuse this peer if the connection says so and it's not marked as inactive
                bool canReuse = (id.Connection?.CanReconnect ?? false) && !id.TorrentManager.InactivePeerManager.InactivePeerList.Contains(id.Uri);
                id.TorrentManager.PieceManager.Picker.CancelRequests(id);
                id.Peer.CleanedUpCount++;

                if (id.PeerExchangeManager != null)
                    id.PeerExchangeManager.Dispose();

                if (!id.AmChoking)
                    id.TorrentManager.UploadingTo--;

                //id.Dispose();

                id.TorrentManager.Peers.ConnectedPeers.RemoveAll(delegate(PeerId other) { return id == other; });

                if (id.TorrentManager.Peers.ActivePeers.Contains(id.Peer))
                    id.TorrentManager.Peers.ActivePeers.Remove(id.Peer);

                // If we get our own details, this check makes sure we don't try connecting to ourselves again
                if (canReuse && id.Peer.PeerId != engine.PeerId)
                {
                    if (!id.TorrentManager.Peers.AvailablePeers.Contains(id.Peer) && id.Peer.CleanedUpCount < 5)
                        id.TorrentManager.Peers.AvailablePeers.Insert(0, id.Peer);
                }
            }
            catch(Exception ex)
            {
                Logger.Log(null, "CleanupSocket Error " + ex.Message);
            }
            finally
            {
                id.TorrentManager.RaisePeerDisconnected(
                    new PeerConnectionEventArgs( id.TorrentManager, id, Direction.None, message ) );
            }

            try
            {
                id.Dispose();
            }
            catch (Exception ex)
            {
                Logger.Log(null, "CleanupSocket Error 2  " + ex.Message);
            }
        }

[alanmcgovern]

Could you submit a pull request with the code changes? That would be much easier for me to review and comment on!

I see your account is quite new so you may not know how. I did a quick search and this looks like it has the steps you'd need to follow to fork the repo, make a change, and then submit a pull request for me to review! https://gist.github.com/Chaser324/ce0505fbed06b947d962 .

If you can't get it to work do let me know and I'll try to copy the changes you've made into a pull request manually.

[OneFingerCodingWarrior]

Hi Alan, sorry for the inconvenience , in fact, I can not check in to the branch monotorrent-0.99.0 with SVN

The modification is not that much , so I think maybe you can copy the changes manually. And I'm not sure if my changes is right, so i need your review

[alanmcgovern]

@OneFingerCodingWarrior With github you can fork the repository, make whatever changes you want to the copy you forked, and then submit those changes to this repository. As such you don't need commit access to this repository in order to change code and propose it!

However, for simplicity i did copy/paste the changes you pasted here and pushed them to a branch to review them! I'll put some comments on that commit as I review :)

[alanmcgovern]

You should be able to see the comments i left here: https://github.com/mono/monotorrent/commit/f0333bb753e2499766610b6d84ddd380db75c23b . Some of the changes look good alright! I'll need to review in a little more detail :)

[alanmcgovern]

I merged a few of the chunks you suggested in your changes! Thanks!