mbed official / USBDevice Featured

USB device stack

Dependents:   mbed-mX-USB-TEST1 USBMSD_SD_HID_HelloWorld HidTest MIDI_usb_bridge ... more

Erik -

Issue: Memory leaks, everywhere (Closed: Fixed)

Problems

The USBDevice library has a bunch of memoryleaks, for the KL25Z, USBMSD and USBSerial.

Problem USBMSD:

In USBMSD.cpp line 120, a significant buffer is allocated. It is nowhere de-allocated.

Solution: Overload the disconnect function of USBDevice.h, free the buffer there. Also add a destructor which calls disconnect.

Problem USBSerial:

In circBuffer.h line 29 something is allocated, never freed, I didn't actually really look into this one, but a search on 'malloc' and 'free' turned it up.

Soluton: Add a destructor which de-allocates it.

Problem KL25:

In USBHAL_KL25.cpp at line 200+ some things are allocated, but also never freed.

Solution: Not a clue :D

Comments

The circbuffer one is I think fairly minor. You should only run into it when you dynamically create and destroy a USBSerial object, I don't think it will be done that often. Although of course if you want to switch between several USB options it is certainly a realistic scenario.

However the KL25 one and the USBMSD one are everytime 'connect' is called an issue. So if you disconnect and connect a USBMSD device a few times (I am trying to let it work together with FATFileSystem, so then one option is to disconnect the device when FATFileSystem is writing) either the KL25 crashes or it fails to connect, depending on when it fails exactly.

Example program is added. Plug in both serial ports of the KL25, use terminal program to listen on the openSDA port @115200 (I think 9600 blocks USB functions too long). It will nicely output when stuff is allocated, and it won't output when it is freed, since it is never freed.

Example program

7 comments:

Erik -
02 Aug 2013

Both for mbed people and other people with the issue: Version with hopefully all memory leaks closed: http://mbed.org/users/Sissors/code/USBDevice/.

I found that not only does connecting/disconnecting result in memory leaks, but in a single call to 'connect', one buffer was allocated 4 times!

Erik -
13 Aug 2013

Btw are those issues actually monitored?

Bogdan Marinescu
10 Sep 2013

Yes, they are! :) I'm looking at them. It's a bit hard to follow, since http://mbed.org/users/Sissors/code/USBDevice/ has a single commit and I can't track the differences introduced by your fix, but I'm getting there.

Erik -
10 Sep 2013

Whoops, guess I converted it to folder and back to library.

With the locations I listed here it should be doable to figure out what is changed ;). Btw I think the fix I used in USBHAL_KL25.cpp isn't the most elegant possibility, but it worked for me.

Bogdan Marinescu
11 Sep 2013

That's exactly what I wanted to ask you :) I wonder if "disconnect" is the right place to free the buffers. Of course it's better than nothing, but I don't know if it's ideal. How did you test your changes?

Erik -
11 Sep 2013

There is an example program included in the issue report, the issue links that at the bottom (granted, it is easy to overlook it): http://mbed.org/users/Sissors/code/USBMSB_KL25Z_memoryleaks/. There the USBDevice library has some printfs, it should work if you just put in my version of the USBDevice library, if you don't do that it should depending on when it runs out of memory display its memory is full or simply crash. I tested it myself on the KL25Z, with less RAM than LPC1768 you don't have to wait as long, also because it has more memory leaks, in a few connects/disconnects it should run out of memory.

That example program also has printfs everytime a buffer is allocated, which is quite often.

I think for the USBMSD code the logical place to de-allocate the buffer is in disconnect, since it allocates it when connecting it makes sense to de-allocate it in disconnect, and then force disconnect to be called in the destructor. Although there is also the question what happens when disconnect or connect is called multiple times in a row. Not something taken care of now by my version of the library. The USBHAL part has a two-fold problem: it allocates memory on connect and doesn't de-allocate it on disconnect/destruction, but also on a single connect it allocates the same pointer multiple times.

I ran into it while making http://mbed.org/users/Sissors/code/USBFileSystem/, combination of FATFileSystem and USBMSD device. When the program is writing to the filesystem it should disconnect the USBMSD device and reconnect later. But it always ran out of memory in a few times, so I started investigating.

Bogdan Marinescu
12 Sep 2013

I've pushed your changes, along with a few minor changes of my own. Thanks!