Opened 10 years ago

Closed 4 months ago

Last modified 4 months ago

#2250 closed defect (fixed)

Memory leak in ftp

Reported by: shaha Owned by: VZ
Priority: normal Milestone:
Component: network Version:
Keywords: wxFTP trivial memory leak Cc: shaha
Blocked By: Blocking:
Patch: yes

Description

Negative CheckCommand in
wxFTP::GetInputStream and
wxFTP::GetOutputStream abandons the socket
pointer resulting in memory leak

Change History (9)

comment:1 Changed 6 years ago by wojdyr

  • Component set to network
  • Keywords wxFTP added

comment:2 Changed 4 months ago by oneeyeman

  • Keywords trivial memory leak added

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

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

comment:3 Changed 4 months ago 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.

comment:4 Changed 4 months ago by oneeyeman

The same is true for GetOutputStream().

Vadim,
Do you want a patch for the fix?

comment:5 follow-up: Changed 4 months ago 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.

comment:6 in reply to: ↑ 5 Changed 4 months ago by oneeyeman

  • Patch set

Vadim,
Replying to 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 )

comment:7 Changed 4 months ago 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.

comment:8 Changed 4 months ago by VZ

  • Owner set to VZ
  • Resolution set to fixed
  • Status changed from new to closed

In 76904:

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

Add the missing delete statements.

Closes #2250.

comment:9 Changed 4 months ago by VZ

In 76905:

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

Add the missing delete statements.

Closes #2250.

Note: See TracTickets for help on using tickets.