[Mono-dev] TcpListener.AcceptTcpClient leaks a socket at each call

[Jonathan Gagnon]

Sorry about the delay.

I didn't find any other references to SetTcpClient so I removed it and I
created a pull request.


*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 Thu, Jan 23, 2014 at 12:34 PM, "Andrés G. Aragoneses"
<knocte at gmail.com>wrote:

> 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"
> > <knocte at gmail.com <mailto:knocte at 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 at lists.ximian.com
> >     <mailto:Mono-devel-list at lists.ximian.com>
> >     http://lists.ximian.com/mailman/listinfo/mono-devel-list
> >
> >
> >
> >
> > _______________________________________________
> > Mono-devel-list mailing list
> > Mono-devel-list at lists.ximian.com
> > http://lists.ximian.com/mailman/listinfo/mono-devel-list
> >
>
>
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ximian.com/pipermail/mono-devel-list/attachments/20140127/f55a9dbc/attachment.html>