Re: #2250: Memory leak in ftp

[wxTrac]

#2250: Memory leak in ftp
----------------------+---------------------------------------
Reporter: shaha | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: network | Version:
Resolution: | Keywords: wxFTP trivial memory leak
Blocked By: | Blocking:
Patch: 0 |
----------------------+---------------------------------------
Changes (by oneeyeman):

* keywords: wxFTP => wxFTP trivial memory leak


Comment:

Memory leak still stands (at least in GetInputStream()).

To fix it just need to call delete on the socket pointer.

--
Ticket URL: <http://trac.wxwidgets.org/ticket/2250#comment:2>

[wxTrac]

#2250: Memory leak in ftp
----------------------+---------------------------------------
Reporter: shaha | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: network | Version:
Resolution: | Keywords: wxFTP trivial memory leak
Blocked By: | Blocking:
Patch: 0 |
----------------------+---------------------------------------

Comment (by oneeyeman):

Just adding following code:


{{{
wxFTP ftp;
if( !ftp.Connect( "ftp.wxwidgets.org" ) )
{
return -1;
}
ftp.ChDir( "/pub/2.8.9" );
wxInputStream *in = ftp.GetInputStream( "abc.doc" );
}}}
will allow to see the leak.

--
Ticket URL: <http://trac.wxwidgets.org/ticket/2250#comment:3>

[wxTrac]

#2250: Memory leak in ftp
----------------------+---------------------------------------
Reporter: shaha | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: network | Version:
Resolution: | Keywords: wxFTP trivial memory leak
Blocked By: | Blocking:
Patch: 0 |
----------------------+---------------------------------------

Comment (by oneeyeman):

The same is true for GetOutputStream().

Vadim,
Do you want a patch for the fix?

--
Ticket URL: <http://trac.wxwidgets.org/ticket/2250#comment:4>

[wxTrac]

#2250: Memory leak in ftp
----------------------+---------------------------------------
Reporter: shaha | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: network | Version:
Resolution: | Keywords: wxFTP trivial memory leak
Blocked By: | Blocking:
Patch: 0 |
----------------------+---------------------------------------

Comment (by vadz):

What is being leaked here exactly? I hope you don't mean that
`wxInputStream` itself is, because this would be hardly surprising
considering you don't delete it, as you're supposed to do.

--
Ticket URL: <http://trac.wxwidgets.org/ticket/2250#comment:5>

[wxTrac]

#2250: Memory leak in ftp
----------------------+---------------------------------------
Reporter: shaha | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: network | Version:
Resolution: | Keywords: wxFTP trivial memory leak
Blocked By: | Blocking:

Patch: 1 |
----------------------+---------------------------------------
Changes (by oneeyeman):

* patch: 0 => 1


Comment:

Vadim,
Replying to [comment:5 vadz]:

> What is being leaked here exactly? I hope you don't mean that
`wxInputStream` itself is, because this would be hardly surprising
considering you don't delete it, as you're supposed to do.

No, of course not.
I'm talking about this:


{{{
diff -bru wxWidgets/src/common/ftp.cpp
/mnt/winxp/wxWidgets/src/common/ftp.cpp
--- wxWidgets/src/common/ftp.cpp 2014-06-14 17:47:19.000000000
-0700
+++ /mnt/winxp/wxWidgets/src/common/ftp.cpp 2014-07-11
02:34:37.546875000 -0700
@@ -769,7 +769,10 @@

wxString tmp_str = wxT("RETR ") + wxURI::Unescape(path);
if ( !CheckCommand(tmp_str, '1') )
+ {
+ delete sock;
return NULL;
+ }

sock = AcceptIfActive(sock);
if ( !sock )

}}}

--
Ticket URL: <http://trac.wxwidgets.org/ticket/2250#comment:6>

[wxTrac]

#2250: Memory leak in ftp
----------------------+---------------------------------------
Reporter: shaha | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: network | Version:
Resolution: | Keywords: wxFTP trivial memory leak
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------------------

Comment (by oneeyeman):

And since GetInputStream() will return NULL in case CheckCommand() will
fail it will not go thru the destructor and will leave undeleted "sock"
pointer.

--
Ticket URL: <http://trac.wxwidgets.org/ticket/2250#comment:7>

[wxTrac]

#2250: Memory leak in ftp
----------------------+---------------------------------------

Reporter: shaha | Owner: VZ
Type: defect | Status: closed

Priority: normal | Milestone:
Component: network | Version:

Resolution: fixed | Keywords: wxFTP trivial memory leak

Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------------------

Changes (by VZ):

* owner: => VZ
* status: new => closed
* resolution: => fixed


Comment:

In [changeset:"76904"]:
{{{
#!CommitTicketReference repository="" revision="76904"
Fix socket leaks in wxFTP if starting up/downloading fails.

Add the missing delete statements.

Closes #2250.
}}}

--
Ticket URL: <http://trac.wxwidgets.org/ticket/2250#comment:8>

[wxTrac]

#2250: Memory leak in ftp
----------------------+---------------------------------------
Reporter: shaha | Owner: VZ
Type: defect | Status: closed
Priority: normal | Milestone:
Component: network | Version:
Resolution: fixed | Keywords: wxFTP trivial memory leak
Blocked By: | Blocking:
Patch: 1 |
----------------------+---------------------------------------

Comment (by VZ):

In [changeset:"76905"]:
{{{
#!CommitTicketReference repository="" revision="76905"

Fix socket leaks in wxFTP if starting up/downloading fails.

Add the missing delete statements.

Closes #2250.
}}}

--

Ticket URL: <http://trac.wxwidgets.org/ticket/2250#comment:9>