Sounds good, then can the SetTcpClient() method be removed? (If it can't because other places in the code are using it, you would need to: - Either make them use the new ctor. - Or fix SetTcpClient() to close the previous socket? )
If it can be removed because there are no more things using it, I say simply create a pull request with your change. On 23/01/14 16:59, Jonathan Gagnon wrote: > Here is the proposed change. See attached files. > > I'm not too familiar with sending diffs so let me know if I didn't send > it in the expected format. > > *Jonathan Gagnon* > Architecte d'application / Application Architect > 600, boulevard Armand-Frappier, bureau 200 > Laval (Québec) H7V 4B4 > Canada > T : 450-662-6101 poste 234 > <http://www.croesus.com> > <http://www.facebook.com/pages/Croesus-Finansoft/345020305606240><http://www.linkedin.com/company/croesus-finansoft?trk=hb_tab_compy_id_26141><https://twitter.com/CroesusFin> > > > > On Wed, Jan 22, 2014 at 5:15 PM, "Andrés G. Aragoneses" > <kno...@gmail.com <mailto:kno...@gmail.com>> wrote: > > On 22/01/14 22:32, Jonathan Gagnon wrote: > > I found a leak in TcpListener.AcceptTcpClient : > > > > public TcpClient AcceptTcpClient () > > { > > if (!active) > > throw new InvalidOperationException ("Socket is not listening"); > > > > Socket clientSocket = server.Accept (); > > > > TcpClient client = new TcpClient(); // this call creates a socket > even > > though we don't need it > > // use internal method SetTcpClient to make a > > // client with the specified socket > > client.SetTcpClient (clientSocket); // This method leaks the socket > > created by the default constructor. > > > > return client; > > } > > > > > > The method calls the default TcpClient constructor which creates a new > > socket. SetTcpClient is then called to assign the accepted socket to > > the TcpClient object. The problem is that the SetTcpClient simply > > replaces the old socket reference by the new without closing the old > > socket. The result is that the socket created by the default > > constructor remains until the GC reclaims it. > > > > The fix would be really easy. Instead of calling the default > TcpClient > > constructor, a new constructor could be created taking the socket as > > parameter. We would then avoid creating and leaking a socket > every time > > the method is called. The fixed method would look like this : > > > > public TcpClient AcceptTcpClient () > > { > > if (!active) > > throw new InvalidOperationException ("Socket is not listening"); > > > > Socket clientSocket = server.Accept (); > > > > TcpClient client = new TcpClient(clientSocket); > > > > return client; > > } > > > > > > I could create a fix with the proposed solution. Any thoughts? > > Propose your solution as diff format please, it's much easier to > understand and review. > > > _______________________________________________ > Mono-devel-list mailing list > Mono-devel-list@lists.ximian.com > <mailto:Mono-devel-list@lists.ximian.com> > http://lists.ximian.com/mailman/listinfo/mono-devel-list > > > > > _______________________________________________ > Mono-devel-list mailing list > Mono-devel-list@lists.ximian.com > http://lists.ximian.com/mailman/listinfo/mono-devel-list > _______________________________________________ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list