hcq/MaxScale
hcq/MaxScale
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MXS-1338: Fix memory leak of after buffer cloning

Browse Source 
When a buffer is cloned and then the original buffer parsed and freed, the
freeing of the cloned buffer will not release the memory that was
allocated when the original buffer is parsed.

This is a side-effect of how the buffer objects are stored in the buffer
and not in the shared memory buffer. The creation of a buffer object after
cloning will cause the buffer object to be lost as the cloned buffer
didn't have a pointer to the buffer object that was created later.

By moving the buffer objects into the shared memory buffer, the memory
leak is fixed.
This commit is contained in:
Markus Mäkelä 2017-08-01 22:41:11 +03:00
parent 0dbf64b1ab
commit 63b16243e7
2 changed files with 21 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
include/maxscale/buffer.h
View File

@ -69,24 +69,13 @@ typedef enum
#define GWBUF_IS_TYPE_RESPONSE_END(b) (b->gwbuf_type & GWBUF_TYPE_RESPONSE_END)
#define GWBUF_IS_TYPE_SESCMD(b) (b->gwbuf_type & GWBUF_TYPE_SESCMD)
/**
* A structure to encapsulate the data in a form that the data itself can be
* shared between multiple GWBUF's without the need to make multiple copies
* but still maintain separate data pointers.
*/
typedef struct
{
unsigned char *data; /*< Physical memory that was allocated */
int refcount; /*< Reference count on the buffer */
} SHARED_BUF;
typedef enum
{
GWBUF_INFO_NONE = 0x0,
GWBUF_INFO_PARSED = 0x1
} gwbuf_info_t;
#define GWBUF_IS_PARSED(b) (b->gwbuf_info & GWBUF_INFO_PARSED)
#define GWBUF_IS_PARSED(b) (b->sbuf->info & GWBUF_INFO_PARSED)
/**
* A structure for cleaning up memory allocations of structures which are
@ -109,6 +98,18 @@ struct buffer_object_st
buffer_object_t* bo_next;
};
/**
* A structure to encapsulate the data in a form that the data itself can be
* shared between multiple GWBUF's without the need to make multiple copies
* but still maintain separate data pointers.
*/
typedef struct
{
unsigned char *data; /*< Physical memory that was allocated */
int refcount; /*< Reference count on the buffer */
buffer_object_t *bufobj; /*< List of objects referred to by GWBUF */
gwbuf_info_t info; /*< Info bits */
} SHARED_BUF;
/**
* The buffer structure used by the descriptor control blocks.
@ -126,8 +127,6 @@ typedef struct gwbuf
void *start; /*< Start of the valid data */
void *end; /*< First byte after the valid data */
SHARED_BUF *sbuf; /*< The shared buffer with the real data */
buffer_object_t *gwbuf_bufobj; /*< List of objects referred to by GWBUF */
gwbuf_info_t gwbuf_info; /*< Info bits */
gwbuf_type_t gwbuf_type; /*< buffer's data type information */
HINT *hint; /*< Hint data for this buffer */
BUF_PROPERTY *properties; /*< Buffer properties */

19
server/core/buffer.c
View File

@ -78,18 +78,19 @@ gwbuf_alloc(unsigned int size)
rval = NULL;
goto retblock;
}
sbuf->refcount = 1;
sbuf->info = GWBUF_INFO_NONE;
sbuf->bufobj = NULL;
spinlock_init(&rval->gwbuf_lock);
rval->start = sbuf->data;
rval->end = (void *)((char *)rval->start + size);
sbuf->refcount = 1;
rval->sbuf = sbuf;
rval->next = NULL;
rval->tail = rval;
rval->hint = NULL;
rval->properties = NULL;
rval->gwbuf_type = GWBUF_TYPE_UNDEFINED;
rval->gwbuf_info = GWBUF_INFO_NONE;
rval->gwbuf_bufobj = NULL;
CHK_GWBUF(rval);
retblock:
if (rval == NULL)
@ -258,7 +259,7 @@ gwbuf_free_one(GWBUF *buf)
{
MXS_FREE(buf->sbuf->data);
MXS_FREE(buf->sbuf);
bo = buf->gwbuf_bufobj;
bo = buf->sbuf->bufobj;
while (bo != NULL)
{
@ -312,8 +313,6 @@ gwbuf_clone_one(GWBUF *buf)
rval->start = buf->start;
rval->end = buf->end;
rval->gwbuf_type = buf->gwbuf_type;
rval->gwbuf_info = buf->gwbuf_info;
rval->gwbuf_bufobj = buf->gwbuf_bufobj;
rval->tail = rval;
rval->next = NULL;
CHK_GWBUF(rval);
@ -376,8 +375,6 @@ static GWBUF *gwbuf_clone_portion(GWBUF *buf,
clonebuf->gwbuf_type = buf->gwbuf_type; /*< clone the type for now */
clonebuf->properties = NULL;
clonebuf->hint = NULL;
clonebuf->gwbuf_info = buf->gwbuf_info;
clonebuf->gwbuf_bufobj = buf->gwbuf_bufobj;
clonebuf->next = NULL;
clonebuf->tail = clonebuf;
CHK_GWBUF(clonebuf);
@ -674,7 +671,7 @@ void gwbuf_add_buffer_object(GWBUF* buf,
newb->bo_next = NULL;
/** Lock */
spinlock_acquire(&buf->gwbuf_lock);
p_b = &buf->gwbuf_bufobj;
p_b = &buf->sbuf->bufobj;
/** Search the end of the list and add there */
while (*p_b != NULL)
{
@ -682,7 +679,7 @@ void gwbuf_add_buffer_object(GWBUF* buf,
}
*p_b = newb;
/** Set flag */
buf->gwbuf_info |= GWBUF_INFO_PARSED;
buf->sbuf->info |= GWBUF_INFO_PARSED;
/** Unlock */
spinlock_release(&buf->gwbuf_lock);
}
@ -694,7 +691,7 @@ void* gwbuf_get_buffer_object_data(GWBUF* buf, bufobj_id_t id)
CHK_GWBUF(buf);
/** Lock */
spinlock_acquire(&buf->gwbuf_lock);
bo = buf->gwbuf_bufobj;
bo = buf->sbuf->bufobj;
while (bo != NULL && bo->bo_id != id)
{