Changeset: 82864c80ce80 for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=82864c80ce80
Modified Files:
gdk/gdk_imprints.c
gdk/gdk_logger.c
gdk/gdk_select.c
monetdb5/extras/rapi/rapi.c
monetdb5/modules/atoms/json.c
monetdb5/modules/kernel/aggr.c
monetdb5/modules/kernel/algebra.c
monetdb5/modules/kernel/bat5.c
monetdb5/modules/kernel/batmmath.c
monetdb5/modules/kernel/batstr.c
monetdb5/modules/kernel/group.c
monetdb5/modules/mal/batExtensions.c
monetdb5/modules/mal/batcalc.c
monetdb5/modules/mal/mat.c
monetdb5/modules/mal/pcre.c
monetdb5/modules/mal/sample.c
monetdb5/modules/mal/txtsim.c
sql/backends/monet5/UDF/pyapi/connection.c
sql/backends/monet5/generator/generator.c
sql/backends/monet5/sql.c
sql/backends/monet5/sql_rank.c
sql/backends/monet5/sql_result.c
sql/backends/monet5/vaults/bam/bam_lib.c
sql/storage/bat/bat_storage.c
Branch: Jul2017
Log Message:
Fix lots of reference counting leaks, especially in error handling.
diffs (truncated from 1696 to 300 lines):
diff --git a/gdk/gdk_imprints.c b/gdk/gdk_imprints.c
--- a/gdk/gdk_imprints.c
+++ b/gdk/gdk_imprints.c
@@ -276,7 +276,7 @@ BATcheckimprints(BAT *b)
gdk_return
BATimprints(BAT *b)
{
- BAT *o = NULL, *s1 = NULL, *s2 = NULL, *s3 = NULL, *s4 = NULL;
+ BAT *s1 = NULL, *s2 = NULL, *s3 = NULL, *s4 = NULL;
Imprints *imprints;
lng t0 = 0;
@@ -305,14 +305,12 @@ BATimprints(BAT *b)
assert(b->timprints == NULL);
if (VIEWtparent(b)) {
- bat p = VIEWtparent(b);
- o = b;
- b = BATdescriptor(p);
+ /* views always keep null pointer and need to obtain
+ * the latest imprint from the parent at query time */
+ b = BBPdescriptor(VIEWtparent(b));
assert(b);
- if (BATcheckimprints(b)) {
- BBPunfix(b->batCacheid);
+ if (BATcheckimprints(b))
return GDK_SUCCEED;
- }
assert(b->timprints == NULL);
}
MT_lock_set(&GDKimprintsLock(b->batCacheid));
@@ -336,9 +334,9 @@ BATimprints(BAT *b)
if (imprints->imprints == NULL ||
(imprints->imprints->filename =
GDKmalloc(strlen(nme) + 12)) == NULL) {
+ MT_lock_unset(&GDKimprintsLock(b->batCacheid));
GDKfree(imprints->imprints);
GDKfree(imprints);
- MT_lock_unset(&GDKimprintsLock(b->batCacheid));
return GDK_FAIL;
}
sprintf(imprints->imprints->filename, "%s.timprints", nme);
@@ -350,6 +348,8 @@ BATimprints(BAT *b)
s1 = BATsample(b, SMP_SIZE);
if (s1 == NULL) {
MT_lock_unset(&GDKimprintsLock(b->batCacheid));
+ GDKfree(imprints->imprints->filename);
+ GDKfree(imprints->imprints);
GDKfree(imprints);
return GDK_FAIL;
}
@@ -357,6 +357,8 @@ BATimprints(BAT *b)
if (s2 == NULL) {
MT_lock_unset(&GDKimprintsLock(b->batCacheid));
BBPunfix(s1->batCacheid);
+ GDKfree(imprints->imprints->filename);
+ GDKfree(imprints->imprints);
GDKfree(imprints);
return GDK_FAIL;
}
@@ -365,6 +367,8 @@ BATimprints(BAT *b)
MT_lock_unset(&GDKimprintsLock(b->batCacheid));
BBPunfix(s1->batCacheid);
BBPunfix(s2->batCacheid);
+ GDKfree(imprints->imprints->filename);
+ GDKfree(imprints->imprints);
GDKfree(imprints);
return GDK_FAIL;
}
@@ -374,6 +378,8 @@ BATimprints(BAT *b)
BBPunfix(s1->batCacheid);
BBPunfix(s2->batCacheid);
BBPunfix(s3->batCacheid);
+ GDKfree(imprints->imprints->filename);
+ GDKfree(imprints->imprints);
GDKfree(imprints);
return GDK_FAIL;
}
@@ -406,10 +412,11 @@ BATimprints(BAT *b)
sizeof(uint64_t) + /* padding for alignment */
pages * sizeof(cchdc_t), /* dict */
1) != GDK_SUCCEED) {
+ MT_lock_unset(&GDKimprintsLock(b->batCacheid));
+ GDKfree(imprints->imprints->filename);
GDKfree(imprints->imprints);
GDKfree(imprints);
GDKerror("#BATimprints: memory allocation error");
- MT_lock_unset(&GDKimprintsLock(b->batCacheid));
BBPunfix(s1->batCacheid);
BBPunfix(s2->batCacheid);
BBPunfix(s3->batCacheid);
@@ -498,22 +505,14 @@ BATimprints(BAT *b)
ALGODEBUG fprintf(stderr, "#BATimprints: imprints construction " LLFMT
" usec\n", GDKusec() - t0);
MT_lock_unset(&GDKimprintsLock(b->batCacheid));
- /* BBPUnfix tries to get the imprints lock which might lead to a
deadlock
- * if those were unfixed earlier */
+ /* BBPUnfix tries to get the imprints lock which might lead to
+ * a deadlock if those were unfixed earlier */
if (s1) {
BBPunfix(s1->batCacheid);
BBPunfix(s2->batCacheid);
BBPunfix(s3->batCacheid);
BBPunfix(s4->batCacheid);
}
- if (o != NULL) {
- o->timprints = NULL; /* views always keep null pointer and
- need to obtain the latest imprint
- from the parent at query time */
- BBPunfix(b->batCacheid);
- b = o;
- }
- assert(b->batCapacity >= BATcount(b));
return GDK_SUCCEED;
}
diff --git a/gdk/gdk_logger.c b/gdk/gdk_logger.c
--- a/gdk/gdk_logger.c
+++ b/gdk/gdk_logger.c
@@ -356,14 +356,17 @@ log_read_updates(logger *lg, trans *tr,
assert(l->nr <= (lng) BUN_MAX);
if (l->flag == LOG_UPDATE) {
uid = COLnew(0, ht, (BUN) l->nr, PERSISTENT);
- if (uid == NULL)
+ if (uid == NULL) {
+ logbat_destroy(b);
return LOG_ERR;
+ }
} else {
assert(ht == TYPE_void);
}
r = COLnew(0, tt, (BUN) l->nr, PERSISTENT);
if (r == NULL) {
BBPreclaim(uid);
+ logbat_destroy(b);
return LOG_ERR;
}
@@ -418,7 +421,6 @@ log_read_updates(logger *lg, trans *tr,
}
if (tv != lg->buf)
GDKfree(tv);
- logbat_destroy(b);
if (res == LOG_OK) {
if (tr_grow(tr) == GDK_SUCCEED) {
@@ -427,6 +429,9 @@ log_read_updates(logger *lg, trans *tr,
tr->changes[tr->nr].ht = ht;
tr->changes[tr->nr].tt = tt;
if ((tr->changes[tr->nr].name =
GDKstrdup(name)) == NULL) {
+ logbat_destroy(b);
+ BBPreclaim(uid);
+ BBPreclaim(r);
return LOG_ERR;
}
tr->changes[tr->nr].b = r;
@@ -440,6 +445,7 @@ log_read_updates(logger *lg, trans *tr,
/* bat missing ERROR or ignore ? currently error. */
res = LOG_ERR;
}
+ logbat_destroy(b);
return res;
}
@@ -902,14 +908,19 @@ logger_open(logger *lg)
GDKdebug &= ~CHECKMASK;
if ((b = BATdescriptor(bid)) == NULL ||
BATmode(b, TRANSIENT) != GDK_SUCCEED ||
- logger_del_bat(lg, bid) != GDK_SUCCEED)
+ logger_del_bat(lg, bid) != GDK_SUCCEED) {
+ logbat_destroy(b);
return GDK_FAIL;
+ }
logbat_destroy(b);
+ b = NULL;
if ((bid = logger_find_bat(lg, "seqs_val")) == 0 ||
(b = BATdescriptor(bid)) == NULL ||
BATmode(b, TRANSIENT) != GDK_SUCCEED ||
- logger_del_bat(lg, bid) != GDK_SUCCEED)
+ logger_del_bat(lg, bid) != GDK_SUCCEED) {
+ logbat_destroy(b);
return GDK_FAIL;
+ }
logbat_destroy(b);
GDKdebug = dbg;
if (bm_commit(lg) != GDK_SUCCEED)
@@ -2759,6 +2770,7 @@ bm_commit(logger *lg)
if ((lb = BATdescriptor(bid)) == NULL ||
BATmode(lb, PERSISTENT) != GDK_SUCCEED) {
+ logbat_destroy(lb);
logbat_destroy(n);
return GDK_FAIL;
}
@@ -2824,6 +2836,7 @@ logger_del_bat(logger *lg, log_bid bid)
assert(p != BUN_NONE);
if (p == BUN_NONE) {
+ logbat_destroy(b);
GDKerror("logger_del_bat: cannot find BAT\n");
return GDK_FAIL;
}
@@ -2833,19 +2846,25 @@ logger_del_bat(logger *lg, log_bid bid)
if (p >= lg->catalog_bid->batInserted &&
(q = log_find(lg->snapshots_bid, lg->dsnapshots, bid)) != BUN_NONE)
{
- if (BUNappend(lg->dsnapshots, &q, FALSE) != GDK_SUCCEED)
+ if (BUNappend(lg->dsnapshots, &q, FALSE) != GDK_SUCCEED) {
+ logbat_destroy(b);
return GDK_FAIL;
+ }
if (lg->debug & 1)
fprintf(stderr,
"#logger_del_bat release snapshot %d (%d)\n",
bid, BBP_lrefs(bid));
- if (BUNappend(lg->freed, &bid, FALSE) != GDK_SUCCEED)
+ if (BUNappend(lg->freed, &bid, FALSE) != GDK_SUCCEED) {
+ logbat_destroy(b);
return GDK_FAIL;
+ }
} else if (p >= lg->catalog_bid->batInserted) {
BBPrelease(bid);
} else {
- if (BUNappend(lg->freed, &bid, FALSE) != GDK_SUCCEED)
+ if (BUNappend(lg->freed, &bid, FALSE) != GDK_SUCCEED) {
+ logbat_destroy(b);
return GDK_FAIL;
+ }
}
if (b) {
lg->changes += BATcount(b) + 1;
diff --git a/gdk/gdk_select.c b/gdk/gdk_select.c
--- a/gdk/gdk_select.c
+++ b/gdk/gdk_select.c
@@ -549,12 +549,12 @@ NAME##_##TYPE(BAT *b, BAT *s, BAT *bn, c
assert(lval); \
assert(hval); \
if (use_imprints && VIEWtparent(b)) { \
- BAT *parent = BATdescriptor(VIEWtparent(b)); \
+ BAT *parent = BBPdescriptor(VIEWtparent(b)); \
+ assert(parent); \
basesrc = (const TYPE *) Tloc(parent, 0); \
imprints = parent->timprints; \
pr_off = (BUN) ((TYPE *)Tloc(b,0) - \
(TYPE *)Tloc(parent,0)); \
- BBPunfix(parent->batCacheid); \
} else { \
imprints = b->timprints; \
basesrc = (const TYPE *) Tloc(b, 0); \
diff --git a/monetdb5/extras/rapi/rapi.c b/monetdb5/extras/rapi/rapi.c
--- a/monetdb5/extras/rapi/rapi.c
+++ b/monetdb5/extras/rapi/rapi.c
@@ -345,6 +345,7 @@ str RAPIeval(Client cntxt, MalBlkPtr mb,
msg = createException(MAL, "rapi.eval",
"Got "BUNFMT"
rows, but can only handle "LLFMT". Sorry.",
BATcount(b),
(lng) RAPI_MAX_TUPLES);
+ BBPunfix(b->batCacheid);
goto wrapup;
}
varname = PROTECT(Rf_install(args[i]));
@@ -493,6 +494,8 @@ void* RAPIloopback(void *query) {
BAT *b = BATdescriptor(output->cols[i].b);
if (b == NULL || !(varvalue = bat_to_sexp(b))) {
UNPROTECT(i + 3);
+ if (b)
+ BBPunfix(b->batCacheid);
return ScalarString(RSTR("Conversion
error"));
}
BBPunfix(b->batCacheid);
diff --git a/monetdb5/modules/atoms/json.c b/monetdb5/modules/atoms/json.c
--- a/monetdb5/modules/atoms/json.c
+++ b/monetdb5/modules/atoms/json.c
@@ -1547,7 +1547,7 @@ JSONvalueArray(json *ret, json *js)
static BAT **
JSONargumentlist(MalBlkPtr mb, MalStkPtr stk, InstrPtr pci)
{
- int i, error = 0, error2 = 0, bats = 0;
+ int i, error = 0, bats = 0;
BUN cnt = 0;
BAT **bl;
@@ -1558,12 +1558,13 @@ JSONargumentlist(MalBlkPtr mb, MalStkPtr
if (isaBatType(getArgType(mb, pci, i))) {
bats++;
bl[i] = BATdescriptor(stk->stk[getArg(pci,
i)].val.bval);
- if (bl[i] == 0)
- error++;
- error2 |= (cnt > 0 && BATcount(bl[i]) != cnt);
+ if (bl[i] == NULL || (cnt > 0 && BATcount(bl[i]) !=
cnt)) {
+ error = 1;
+ break;
+ }
cnt = BATcount(bl[i]);
}
- if (error + error2 || bats == 0) {
+ if (error || bats == 0) {
for (i = pci->retc; i < pci->argc; i++)
if (bl[i])
BBPunfix(bl[i]->batCacheid);
@@ -2427,19 +2428,12 @@ JSONsubjsoncand(bat *retval, bat *bid, b
_______________________________________________
checkin-list mailing list
[email protected]
https://www.monetdb.org/mailman/listinfo/checkin-list
