1 Jun 2006 03:14
Re: SOLVED: Memory leak in usb_os_find_devices() on darwin
Caribe Schreiber <caribe <at> candcsolution.com>
2006-06-01 01:14:03 GMT
2006-06-01 01:14:03 GMT
Hello all,
I made some very minor changes and have a patch for the 1.1.10a version as well as the 1.1.12 version. Both patches are enclosed below.
~Caribe
Version:1.1.10a File:usb.c
___begin___
14a15
> #include "config.h"
293a295,299
>
> #ifdef DARWIN_API
> //we need to free the device address data that is allocated in the darwin port here
> free(dev->dev);
> #endif
___end___
Version 1.1.12 File:usb.c
___begin___
15a16,17
> #include "config.h"
>
294a297,300
> #ifdef DARWIN_API
> //we need to free the device address data that is allocated in the darwin port here
> free(dev->dev);
> #endif
___end___
On May 31, 2006, at 11:56 AM, Caribe Schreiber wrote:
Hello all,I noted that there appears to be a memory leak in the usb_os_find_devices() function in the darwin port. The function seems to leak several 4 byte allocations each time it's called (looks like <num_devices_on_bus> + 1 allocations are leaked). Has anyone else noted this? It seems like the problem lies with this alloc on line 1288 of darwin.c:dev->dev = (USBDeviceAddress *)malloc(4);memcpy(dev->dev, &location, 4);but I don't know the internals well enough to know if they're getting freed somewhere else (or more accurately where they should be free()'d). When I change the malloc to do a 5 byte malloc the leaked segments are increased to 5 bytes, so this is definitely the culprit.I've been working with the 1.1.10a tree since it seems that parts of the 1.1.12 darwin implementation are broken (namely bulk_read()), but I've tried this against 1.1.12 as well and found the same leak behavior.Are there any darwin gurus out there that can shed some light on this behavior?~Caribe===================Caribe SchreiberIT EngineerC&C Solutions, Inc.Quote of the week:"He has never been known to use a word that might send a reader to the dictionary."--William Faulkner (about Ernest Hemingway)"Poor Faulkner. Does he really think big emotions come from big words?"--Ernest Hemingway (about William Faulkner)
===================
Caribe Schreiber
IT Engineer
C&C Solutions, Inc.
Quote of the week:
"He has never been known to use a word that might send a reader to the dictionary."
--William Faulkner (about Ernest Hemingway)
"Poor Faulkner. Does he really think big emotions come from big words?"
--Ernest Hemingway (about William Faulkner)
- Chris
Index: linux.c
===================================================================
--- linux.c (revision 640)
+++ linux.c (working copy)
<at> <at> -346,6 +346,54 <at> <at>
return 0;
}
+static struct usbi_io* linux_find_usbi_io(struct usbi_dev_handle *hdev, unsigned int tag)
+{
+ struct usbi_io *io;
+ list_for_each_entry(io, &hdev->ios, list) {
+ unsigned int iotag;
+ switch( io->type )
+ {
+ case USBI_IO_CONTROL:
+ iotag = io->ctrl.request->tag;
+ break;
+
+ case USBI_IO_INTERRUPT:
+ iotag = io->intr.request->tag;
+ break;
+
+ case USBI_IO_BULK:
+ iotag = io->bulk.request->tag;
+ break;
+
+ case USBI_IO_ISOCHRONOUS:
+ iotag = io->isoc.request->tag;
+ break;
+
+ default:
+ /* make sure it doesn't match */
+ iotag = ~tag;
+ break;
+ }
+
+ if( iotag == tag )
+ return io;
+ }
+ return 0;
+}
+
+static int linux_io_wait(struct usbi_dev_handle *hdev, unsigned int tag)
+{
+ struct usbi_io *io = hdev->idev->ops->find_usbi_io(hdev, tag);
+ if( !io ) {
+ pthread_mutex_lock(&io->lock);
+ if( io->inprogress )
+ pthread_cond_wait(&io->cond, &io->lock);
+ pthread_mutex_unlock(&io->lock);
+ return LIBUSB_SUCCESS;
+ }
+ return LIBUSB_UNKNOWN_TAG;
+}
+
static void *poll_events(void *unused)
{
char filename[PATH_MAX + 1];
<at> <at> -1019,7 +1067,9 <at> <at>
.submit_intr = linux_submit_intr,
.submit_bulk = linux_submit_bulk,
.submit_isoc = linux_submit_isoc,
+ .find_usbi_io = linux_find_usbi_io,
.io_cancel = linux_io_cancel,
+ .io_wait = linux_io_wait,
},
};
Index: usbi.h
===================================================================
--- usbi.h (revision 640)
+++ usbi.h (working copy)
<at> <at> -256,7 +256,9 <at> <at>
int (*submit_intr)(struct usbi_dev_handle *hdev, struct usbi_io *io);
int (*submit_bulk)(struct usbi_dev_handle *hdev, struct usbi_io *io);
int (*submit_isoc)(struct usbi_dev_handle *hdev, struct usbi_io *io);
+ struct usbi_io* (*find_usbi_io)(struct usbi_dev_handle *hdev, unsigned int tag);
int (*io_cancel)(struct usbi_io *io);
+ int (*io_wait)(struct usbi_dev_handle *hdev, unsigned int tag);
};
struct usbi_backend_ops {
Index: api.c
===================================================================
--- api.c (revision 640)
+++ api.c (working copy)
<at> <at> -107,3 +107,50 <at> <at>
return hdev->idev->ops->detach_kernel_driver_np(hdev, interface);
}
+int libusb_abort(libusb_dev_handle_t dev, unsigned int tag)
+{
+ struct usbi_dev_handle *hdev;
+
+ hdev = usbi_find_dev_handle(dev);
+ if (!hdev)
+ return LIBUSB_UNKNOWN_DEVICE;
+
+ struct usbi_io *io = hdev->idev->ops->find_usbi_io(hdev, tag);
+ if (!hdev)
+ return LIBUSB_UNKNOWN_TAG;
+
+ return hdev->idev->ops->io_cancel(io);
+}
+
+int libusb_wait(libusb_dev_handle_t dev, unsigned int tag)
+{
+ struct usbi_dev_handle *hdev;
+
+ hdev = usbi_find_dev_handle(dev);
+ if (!hdev)
+ return LIBUSB_UNKNOWN_DEVICE;
+
+ return hdev->idev->ops->io_wait(hdev, tag);
+}
+
+int libusb_poll(libusb_dev_handle_t dev, unsigned int num_tags,
+ unsigned int *tags, unsigned int *tag)
+{
+ struct usbi_dev_handle *hdev;
+ unsigned int i;
+
+ hdev = usbi_find_dev_handle(dev);
+ if (!hdev)
+ return LIBUSB_UNKNOWN_DEVICE;
+
+ for( i = 0; i < num_tags; i++ ) {
+ struct usbi_io *io = hdev->idev->ops->find_usbi_io(hdev, tags[i]);
+ if( io && !io->inprogress ) {
+ *tag = tags[i];
+ return LIBUSB_SUCCESS;
+ }
+ }
+
+ return LIBUSB_FAILURE;
+}
+
Index: libusb.h.in
===================================================================
--- libusb.h.in (revision 640)
+++ libusb.h.in (working copy)
<at> <at> -183,6 +183,7 <at> <at>
#define LIBUSB_NOACCESS -9 /* Access to device denied */
#define LIBUSB_PARSE_ERROR -10 /* Data could not be parsed */
#define LIBUSB_UNKNOWN_DEVICE -11 /* Device id is stale or invalid */
+#define LIBUSB_UNKNOWN_TAG -12 /* Tag ID not found */
#define LIBUSB_IO_STALL -50 /* Endpoint stalled */
#define LIBUSB_IO_CRC_ERROR -51 /* CRC error */
<at> <at> -797,6 +798,7 <at> <at>
*
* Arguments:
* usb_ctrl_req - Pointer to USB control request
+ * request object must exist for the lifetime of IO
* residue - transfer residue
* callback - callback handler
* arg - caller defined
<at> <at> -839,6 +841,7 <at> <at>
* Arguments:
* libusb_intr_req - Pointer to USB intr request
* (EP direction determines R/W)
+ * request object must exist for the lifetime of IO
* residue - transfer residue
* callback - callback handler
* arg - caller defined
<at> <at> -880,6 +883,7 <at> <at>
* Arguments:
* libusb_bulk_req - Pointer to USB bulk request
* (EP direction determines R/W)
+ * request object must exist for the lifetime of IO
* residue - transfer residue
* callback - callback handler
* arg - caller defined
<at> <at> -930,6 +934,7 <at> <at>
* Arguments:
* libusb_isoc_req - Pointer to USB isoc request
* (EP direction determines R/W)
+ * request object must exist for the lifetime of IO
* residue - transfer residue
* callback - callback handler
* arg - caller defined
<at> <at> -984,7 +989,7 <at> <at>
*
* NOT Implemented
*/
-int libusb_abort(unsigned int tag);
+int libusb_abort(libusb_dev_handle_t dev, unsigned int tag);
/*
* I/O Support:
<at> <at> -1006,10 +1011,9 <at> <at>
*
* NOT Implemented
*/
-int libusb_wait(unsigned int num_tags, unsigned int *tags,
- unsigned int *tag);
-int libusb_poll(unsigned int num_tags, unsigned int *tags,
- unsigned int *tag);
+int libusb_wait(libusb_dev_handle_t dev, unsigned int tag);
+int libusb_poll(libusb_dev_handle_t dev, unsigned int num_tags,
+ unsigned int *tags, unsigned int *tag);
#ifdef __cplusplus
}
Index: async.c
===================================================================
--- async.c (revision 640)
+++ async.c (working copy)
<at> <at> -93,19 +93,6 <at> <at>
/* Helper routine. To be called from the various ports */
void usbi_io_complete(struct usbi_io *io, int status, size_t transferred_bytes)
{
- pthread_mutex_lock(&io->lock);
- io->inprogress = 0;
- pthread_mutex_unlock(&io->lock);
-
- /* Add completion for later retrieval */
- pthread_mutex_lock(&completion_lock);
- list_add(&io->list, &completions);
- pthread_mutex_unlock(&completion_lock);
-
- pthread_mutex_lock(&io->lock);
- pthread_cond_broadcast(&io->cond);
- pthread_mutex_unlock(&io->lock);
-
switch (io->type) {
case USBI_IO_CONTROL:
io->ctrl.callback(io->ctrl.request, io->ctrl.arg, status, transferred_bytes);
<at> <at> -120,6 +107,19 <at> <at>
/* FIXME: Implement */
break;
}
+
+ pthread_mutex_lock(&io->lock);
+ io->inprogress = 0;
+ pthread_mutex_unlock(&io->lock);
+
+ /* Add completion for later retrieval */
+ pthread_mutex_lock(&completion_lock);
+ list_add(&io->list, &completions);
+ pthread_mutex_unlock(&completion_lock);
+
+ pthread_mutex_lock(&io->lock);
+ pthread_cond_broadcast(&io->cond);
+ pthread_mutex_unlock(&io->lock);
}
/*
RSS Feed