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

["Andrés G. Aragoneses"]

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
>