Changeset: edf65208c59d for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB/rev/edf65208c59d
Modified Files:
gdk/gdk_heap.c
sql/backends/monet5/UDF/pyapi3/conversion.h
sql/backends/monet5/UDF/pyapi3/conversion3.c
sql/backends/monet5/UDF/pyapi3/convert_loops.h
sql/backends/monet5/UDF/pyapi3/pyapi3.c
Branch: default
Log Message:
Fix leakage in pyapi, both observed and potential.
diffs (231 lines):
diff --git a/gdk/gdk_heap.c b/gdk/gdk_heap.c
--- a/gdk/gdk_heap.c
+++ b/gdk/gdk_heap.c
@@ -45,6 +45,7 @@
#include "monetdb_config.h"
#include "gdk.h"
#include "gdk_private.h"
+#include "gdk_interprocess.h"
#include "mutils.h"
static void *
@@ -606,7 +607,7 @@ void
HEAPfree(Heap *h, bool rmheap)
{
if (h->base) {
- if (h->farmid == 1) {
+ if (h->farmid == 1 && (h->storage == STORE_MEM || h->storage ==
STORE_MMAP || h->storage == STORE_PRIV)) {
QryCtx *qc = MT_thread_get_qry_ctx();
if (qc)
ATOMIC_SUB(&qc->datasize, h->size);
@@ -617,7 +618,11 @@ HEAPfree(Heap *h, bool rmheap)
} else if (h->storage == STORE_CMEM) {
//heap is stored in regular C memory rather than GDK
memory,so we call free()
free(h->base);
- } else { /* mapped file, or STORE_PRIV */
+ } else if (h->storage == STORE_MMAPABS) {
+ size_t id;
+ sscanf(h->filename, "%zu", &id);
+ GDKreleasemmap(h->base, h->size, id);
+ } else if (h->storage != STORE_NOWN) { /* mapped file, or
STORE_PRIV */
gdk_return ret = GDKmunmap(h->base, h->size);
if (ret != GDK_SUCCEED) {
diff --git a/sql/backends/monet5/UDF/pyapi3/conversion.h
b/sql/backends/monet5/UDF/pyapi3/conversion.h
--- a/sql/backends/monet5/UDF/pyapi3/conversion.h
+++ b/sql/backends/monet5/UDF/pyapi3/conversion.h
@@ -22,6 +22,7 @@ typedef struct {
PyObject *numpy_mask; // PyArrayObject* with mask (NULL if there is no
mask)
void *array_data; // void* pointer to data
bool *mask_data; // bool* pointer to mask data
+ size_t array_size;
size_t count; // amount of return elements
size_t memory_size; // memory size of each element
lng mmap_id;
@@ -32,6 +33,7 @@ typedef struct {
typedef struct {
void *dataptr; // pointer to input data
BAT *bat; // pointer to input BAT
+ BAT *conv_bat; // converted input BAT
int bat_type; // BAT type as TYPE_<type>
sql_subtype *sql_subtype; // SQL typename (for _column_types)
size_t count; // amount of elements in BAT
diff --git a/sql/backends/monet5/UDF/pyapi3/conversion3.c
b/sql/backends/monet5/UDF/pyapi3/conversion3.c
--- a/sql/backends/monet5/UDF/pyapi3/conversion3.c
+++ b/sql/backends/monet5/UDF/pyapi3/conversion3.c
@@ -201,7 +201,7 @@ PyObject *PyArrayObject_FromBAT(PyInput
goto wrapup;
} else {
BAT *ret_bat = NULL;
- msg = ConvertFromSQLType(inp->bat, inp->sql_subtype,
&ret_bat,
+ msg = ConvertFromSQLType(b, inp->sql_subtype, &ret_bat,
&inp->bat_type);
if (msg != MAL_SUCCEED) {
freeException(msg);
@@ -500,7 +500,7 @@ PyObject *PyArrayObject_FromBAT(PyInput
goto wrapup;
}
if (b != inp->bat)
- BBPunfix(b->batCacheid);
+ inp->conv_bat = b; /* still in use, free later */
return vararray;
wrapup:
*return_message = msg;
@@ -1131,6 +1131,7 @@ BAT *PyObject_ConvertToBAT(PyReturn *ret
if (ConvertableSQLType(type)) {
BAT *result;
msg = ConvertToSQLType(NULL, b, type, &result, &bat_type);
+ BBPunfix(b->batCacheid);
if (msg != MAL_SUCCEED) {
goto wrapup;
}
@@ -1242,6 +1243,7 @@ str ConvertFromSQLType(BAT *b, sql_subty
}
if (res == MAL_SUCCEED) {
*ret_bat = BATdescriptor(result);
+ BBPrelease(result);
*ret_type = TYPE_dbl;
} else {
*ret_bat = NULL;
diff --git a/sql/backends/monet5/UDF/pyapi3/convert_loops.h
b/sql/backends/monet5/UDF/pyapi3/convert_loops.h
--- a/sql/backends/monet5/UDF/pyapi3/convert_loops.h
+++ b/sql/backends/monet5/UDF/pyapi3/convert_loops.h
@@ -71,11 +71,14 @@
#if defined(HAVE_FORK)
#define CREATE_BAT_ZEROCOPY(bat, mtpe, batstore)
\
{
\
- bat = COLnew(seqbase, TYPE_##mtpe, 0, TRANSIENT);
\
+ bat = COLnew(seqbase, TYPE_void, 0, TRANSIENT);
\
if (bat == NULL) {
\
msg = createException(MAL, "pyapi3.eval",
SQLSTATE(PY000) "Cannot create column"); \
goto wrapup;
\
}
\
+ bat->ttype = TYPE_##mtpe;
\
+ bat->twidth = ATOMsize(TYPE_##mtpe);
\
+ bat->tshift = ATOMelmshift(b->twidth);
\
bat->tnil = false;
\
bat->tnonil = true;
\
bat->tkey = false;
\
@@ -97,31 +100,26 @@
nancheck_##mtpe(bat);
\
}
\
\
- /*When we create a BAT a small part of memory is allocated,
free it*/ \
- GDKfree(bat->theap->base);
\
+ assert(bat->theap->base == NULL);
\
bat->theap->base =
\
&data[(index_offset * ret->count) * ret->memory_size];
\
- bat->theap->size = ret->count * ret->memory_size;
\
- bat->theap->free =
\
- bat->theap->size; /*There are no free places in the
array*/ \
+ bat->theap->free = ret->count * ret->memory_size;
\
bat->theap->dirty = true;
\
/*If index_offset > 0, we are mapping part of a
multidimensional */ \
/* array.*/
\
/*The entire array will be cleared when the part with
index_offset=0 */\
/* is freed*/
\
/*So we set this part of the mapping to 'NOWN'*/
\
- if (index_offset > 0)
\
+ if (index_offset > 0) {
\
+ bat->theap->size = bat->theap->free;
\
bat->theap->storage = STORE_NOWN;
\
- else {
\
+ } else {
\
+ bat->theap->size = ret->array_size ? ret->array_size :
bat->theap->free; \
bat->theap->storage = batstore;
\
if (batstore == STORE_MMAPABS) {
\
- /* If we are taking data from a MMAP file, set
the filename to \
- * the absolute path */
\
- char address[sizeof(bat->theap->filename)-4];
\
- GDKmmapfile(address, sizeof(address),
ret->mmap_id); \
- strconcat_len(bat->theap->filename,
\
-
sizeof(bat->theap->filename), \
- address, ".tmp",
NULL); \
+ /* Communicate the id to the GDK layer */
\
+ snprintf(bat->theap->filename,
sizeof(bat->theap->filename), \
+ LLFMT, ret->mmap_id);
\
ret->mmap_id = -1;
\
}
\
}
\
@@ -137,11 +135,14 @@
#else
#define CREATE_BAT_ZEROCOPY(bat, mtpe, batstore)
\
{
\
- bat = COLnew(seqbase, TYPE_##mtpe, 0, TRANSIENT);
\
+ bat = COLnew(seqbase, TYPE_void, 0, TRANSIENT);
\
if (bat == NULL) {
\
msg = createException(MAL, "pyapi3.eval",
SQLSTATE(PY000) "Cannot create column"); \
goto wrapup;
\
}
\
+ bat->ttype = TYPE_##mtpe;
\
+ bat->twidth = ATOMsize(TYPE_##mtpe);
\
+ bat->tshift = ATOMelmshift(b->twidth);
\
bat->tnil = false;
\
bat->tnonil = true;
\
bat->tkey = false;
\
@@ -162,22 +163,21 @@
bat->tnonil = false;
\
nancheck_##mtpe(bat);
\
}
\
- /*When we create a BAT a small part of memory is allocated,
free it*/ \
- GDKfree(bat->theap->base);
\
+ assert(bat->theap->base == NULL);
\
bat->theap->base =
\
&data[(index_offset * ret->count) * ret->memory_size];
\
- bat->theap->size = ret->count * ret->memory_size;
\
- bat->theap->free =
\
- bat->theap->size; /*There are no free places in the
array*/ \
+ bat->theap->free = ret->count * ret->memory_size;
\
bat->theap->dirty = true;
\
/*If index_offset > 0, we are mapping part of a
multidimensional */ \
/* array.*/
\
/*The entire array will be cleared when the part with
index_offset=0 */\
/* is freed*/
\
/*So we set this part of the mapping to 'NOWN'*/
\
- if (index_offset > 0)
\
+ if (index_offset > 0) {
\
+ bat->theap->size = bat->theap->free;
\
bat->theap->storage = STORE_NOWN;
\
- else {
\
+ } else {
\
+ bat->theap->size = ret->array_size ? ret->array_size :
bat->theap->free; \
bat->theap->storage = batstore;
\
}
\
bat->theap->newstorage = STORE_MEM;
\
diff --git a/sql/backends/monet5/UDF/pyapi3/pyapi3.c
b/sql/backends/monet5/UDF/pyapi3/pyapi3.c
--- a/sql/backends/monet5/UDF/pyapi3/pyapi3.c
+++ b/sql/backends/monet5/UDF/pyapi3/pyapi3.c
@@ -183,7 +183,7 @@ static str PyAPIeval(Client cntxt, MalBl
char *mmap_ptr;
QueryStruct *query_ptr = NULL;
int query_sem = -1;
- int mmap_id = -1;
+ lng mmap_id = -1;
size_t memory_size = 0;
bool child_process = false;
bool holds_gil = !mapped;
@@ -528,6 +528,7 @@ static str PyAPIeval(Client cntxt, MalBl
goto wrapup;
}
ret->array_data = mmap_ptrs[i + 3];
+ ret->array_size = mmap_sizes[i + 3];
ret->mask_data = NULL;
ret->numpy_array = NULL;
ret->numpy_mask = NULL;
@@ -1151,6 +1152,7 @@ static str PyAPIeval(Client cntxt, MalBl
ret->count = PyArray_DIMS((PyArrayObject
*)ret->numpy_array)[0];
ret->array_data =
PyArray_DATA((PyArrayObject
*)ret->numpy_array);
+ ret->array_size = ret->memory_size * ret->count;
}
descr->npy_type = ret->result_type;
@@ -1338,6 +1340,8 @@ wrapup:
PyInput *inp = &pyinput_values[i - (pci->retc + 2 +
has_card_arg)];
if (inp->bat != NULL)
BBPunfix(inp->bat->batCacheid);
+ if (inp->conv_bat != NULL)
+ BBPunfix(inp->conv_bat->batCacheid); /* delayed free */
}
if (pResult != NULL && gstate == 0) {
// if there is a pResult here, we are running single threaded
(LANGUAGE
_______________________________________________
checkin-list mailing list -- checkin-list@monetdb.org
To unsubscribe send an email to checkin-list-le...@monetdb.org
