greg/wterl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

* Check to make sure that all ASYNC_NIF_REPLY() calls are at the end of their

Browse source 
blocks, just before return and after releasing any local resources or locks.

* Check the return value of when setting up cursor caches so as not to miss an
ENOMEM or other error.

*Cleanup and free resources when closing a connection handle.

* Add a few missing mutex unlock calls on error paths.

* Ensure all resources are free'd/released/closed during truncate error paths.

* Free up alloc'ed copies of keys, cursor handles and sessions on unload.
This commit is contained in:
Gregory Burd 2013-04-20 07:38:11 -04:00
parent 01a79a08c0
commit 3310129918
1 changed files with 97 additions and 63 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

130
c_src/wterl.c
View file

@ -217,8 +217,10 @@ __retain_cursor(WterlConnHandle *conn_handle, unsigned int worker_id, const char
if (ctx->session == NULL) { if (ctx->session == NULL) {
enif_mutex_lock(conn_handle->contexts_mutex); enif_mutex_lock(conn_handle->contexts_mutex);
__init_session_and_cursor_cache(conn_handle, ctx); // TODO: check return value rc = __init_session_and_cursor_cache(conn_handle, ctx);
enif_mutex_unlock(conn_handle->contexts_mutex); enif_mutex_unlock(conn_handle->contexts_mutex);
if (rc != 0)
return rc;
} }
h = ctx->cursors; h = ctx->cursors;
@ -351,6 +353,8 @@ ASYNC_NIF_DECL(
memset(conn_handle->contexts, 0, sizeof(WterlCtx) * ASYNC_NIF_MAX_WORKERS); memset(conn_handle->contexts, 0, sizeof(WterlCtx) * ASYNC_NIF_MAX_WORKERS);
ERL_NIF_TERM result = enif_make_resource(env, conn_handle); ERL_NIF_TERM result = enif_make_resource(env, conn_handle);
/* Keep track of open connections so as to free when unload/reload/etc.
are called. */
khash_t(conns) *h; khash_t(conns) *h;
enif_mutex_lock(args->priv->conns_mutex); enif_mutex_lock(args->priv->conns_mutex);
h = args->priv->conns; h = args->priv->conns;
@ -361,8 +365,8 @@ ASYNC_NIF_DECL(
enif_mutex_unlock(args->priv->conns_mutex); enif_mutex_unlock(args->priv->conns_mutex);
enif_release_resource(conn_handle); enif_release_resource(conn_handle);
ASYNC_NIF_REPLY(enif_make_tuple2(env, ATOM_OK, result));
enif_mutex_unlock(conn_handle->contexts_mutex); enif_mutex_unlock(conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(enif_make_tuple2(env, ATOM_OK, result));
} }
else else
{ {
@ -407,6 +411,7 @@ ASYNC_NIF_DECL(
WT_CONNECTION* conn = args->conn_handle->conn; WT_CONNECTION* conn = args->conn_handle->conn;
int rc = conn->close(conn, NULL); int rc = conn->close(conn, NULL);
/* Connection is closed, remove it so we don't free on unload/reload/etc. */
khash_t(conns) *h; khash_t(conns) *h;
enif_mutex_lock(args->priv->conns_mutex); enif_mutex_lock(args->priv->conns_mutex);
h = args->priv->conns; h = args->priv->conns;
@ -418,8 +423,9 @@ ASYNC_NIF_DECL(
kh_value(h, itr) = NULL; kh_value(h, itr) = NULL;
} }
enif_mutex_unlock(args->priv->conns_mutex); enif_mutex_unlock(args->priv->conns_mutex);
// TODO: dtor? enif_mutex_destroy(args->conn_handle->contexts_mutex);
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
enif_mutex_destroy(args->conn_handle->contexts_mutex);
memset(args->conn_handle, 0, sizeof(WterlConnHandle));
ASYNC_NIF_REPLY(rc == 0 ? ATOM_OK : __strerror_term(env, rc)); ASYNC_NIF_REPLY(rc == 0 ? ATOM_OK : __strerror_term(env, rc));
}, },
@ -519,8 +525,8 @@ ASYNC_NIF_DECL(
ErlNifBinary config; ErlNifBinary config;
if (!enif_inspect_binary(env, args->config, &config)) { if (!enif_inspect_binary(env, args->config, &config)) {
ASYNC_NIF_REPLY(enif_make_badarg(env));
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(enif_make_badarg(env));
return; return;
} }
@ -531,8 +537,8 @@ ASYNC_NIF_DECL(
WT_SESSION *session = NULL; WT_SESSION *session = NULL;
int rc = conn->open_session(conn, NULL, args->conn_handle->session_config, &session); int rc = conn->open_session(conn, NULL, args->conn_handle->session_config, &session);
if (rc != 0) { if (rc != 0) {
ASYNC_NIF_REPLY(__strerror_term(env, rc));
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(__strerror_term(env, rc));
return; return;
} }
/* Note: we locked the context mutex and called __close_cursors_on() /* Note: we locked the context mutex and called __close_cursors_on()
@ -586,6 +592,7 @@ ASYNC_NIF_DECL(
ErlNifBinary config; ErlNifBinary config;
if (!enif_inspect_binary(env, args->config, &config)) { if (!enif_inspect_binary(env, args->config, &config)) {
enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(enif_make_badarg(env)); ASYNC_NIF_REPLY(enif_make_badarg(env));
return; return;
} }
@ -597,6 +604,7 @@ ASYNC_NIF_DECL(
WT_SESSION *session = NULL; WT_SESSION *session = NULL;
int rc = conn->open_session(conn, NULL, args->conn_handle->session_config, &session); int rc = conn->open_session(conn, NULL, args->conn_handle->session_config, &session);
if (rc != 0) { if (rc != 0) {
enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(__strerror_term(env, rc)); ASYNC_NIF_REPLY(__strerror_term(env, rc));
return; return;
} }
@ -652,6 +660,7 @@ ASYNC_NIF_DECL(
ErlNifBinary config; ErlNifBinary config;
if (!enif_inspect_binary(env, args->config, &config)) { if (!enif_inspect_binary(env, args->config, &config)) {
enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(enif_make_badarg(env)); ASYNC_NIF_REPLY(enif_make_badarg(env));
return; return;
} }
@ -663,6 +672,7 @@ ASYNC_NIF_DECL(
WT_SESSION *session = NULL; WT_SESSION *session = NULL;
int rc = conn->open_session(conn, NULL, args->conn_handle->session_config, &session); int rc = conn->open_session(conn, NULL, args->conn_handle->session_config, &session);
if (rc != 0) { if (rc != 0) {
enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(__strerror_term(env, rc)); ASYNC_NIF_REPLY(__strerror_term(env, rc));
return; return;
} }
@ -782,8 +792,8 @@ ASYNC_NIF_DECL(
ErlNifBinary config; ErlNifBinary config;
if (!enif_inspect_binary(env, args->config, &config)) { if (!enif_inspect_binary(env, args->config, &config)) {
ASYNC_NIF_REPLY(enif_make_badarg(env));
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(enif_make_badarg(env));
return; return;
} }
@ -795,8 +805,8 @@ ASYNC_NIF_DECL(
WT_SESSION *session = NULL; WT_SESSION *session = NULL;
int rc = conn->open_session(conn, NULL, args->conn_handle->session_config, &session); int rc = conn->open_session(conn, NULL, args->conn_handle->session_config, &session);
if (rc != 0) { if (rc != 0) {
ASYNC_NIF_REPLY(__strerror_term(env, rc));
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(__strerror_term(env, rc));
return; return;
} }
@ -810,26 +820,26 @@ ASYNC_NIF_DECL(
mess. */ mess. */
if (!args->from_first) { if (!args->from_first) {
if (!enif_inspect_binary(env, args->start, &start_key)) { if (!enif_inspect_binary(env, args->start, &start_key)) {
ASYNC_NIF_REPLY(enif_make_badarg(env));
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(enif_make_badarg(env));
return; return;
} }
} }
rc = session->open_cursor(session, args->uri, NULL, "raw", &start); rc = session->open_cursor(session, args->uri, NULL, "raw", &start);
if (rc != 0) { if (rc != 0) {
ASYNC_NIF_REPLY(__strerror_term(env, rc));
session->close(session, NULL); session->close(session, NULL);
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(__strerror_term(env, rc));
return; return;
} }
/* Position the start cursor at the first record or the specified record. */ /* Position the start cursor at the first record or the specified record. */
if (args->from_first) { if (args->from_first) {
rc = start->next(start); rc = start->next(start);
if (rc != 0) { if (rc != 0) {
ASYNC_NIF_REPLY(__strerror_term(env, rc));
start->close(start); start->close(start);
session->close(session, NULL); session->close(session, NULL);
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(__strerror_term(env, rc));
return; return;
} }
} else { } else {
@ -839,38 +849,40 @@ ASYNC_NIF_DECL(
start->set_key(start, item_start); start->set_key(start, item_start);
rc = start->search(start); rc = start->search(start);
if (rc != 0) { if (rc != 0) {
ASYNC_NIF_REPLY(__strerror_term(env, rc));
start->close(start); start->close(start);
session->close(session, NULL); session->close(session, NULL);
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(__strerror_term(env, rc));
return; return;
} }
} }
if (!args->to_last) { if (!args->to_last) {
if (!enif_inspect_binary(env, args->stop, &stop_key)) { if (!enif_inspect_binary(env, args->stop, &stop_key)) {
ASYNC_NIF_REPLY(enif_make_badarg(env)); start->close(start);
session->close(session, NULL);
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(enif_make_badarg(env));
return; return;
} }
} }
rc = session->open_cursor(session, args->uri, NULL, "raw", &stop); rc = session->open_cursor(session, args->uri, NULL, "raw", &stop);
if (rc != 0) { if (rc != 0) {
ASYNC_NIF_REPLY(__strerror_term(env, rc));
start->close(start); start->close(start);
session->close(session, NULL); session->close(session, NULL);
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(__strerror_term(env, rc));
return; return;
} }
/* Position the stop cursor at the last record or the specified record. */ /* Position the stop cursor at the last record or the specified record. */
if (args->to_last) { if (args->to_last) {
rc = stop->prev(stop); rc = stop->prev(stop);
if (rc != 0) { if (rc != 0) {
ASYNC_NIF_REPLY(__strerror_term(env, rc));
start->close(start); start->close(start);
stop->close(stop); stop->close(stop);
session->close(session, NULL); session->close(session, NULL);
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(__strerror_term(env, rc));
return; return;
} }
} else { } else {
@ -880,11 +892,11 @@ ASYNC_NIF_DECL(
stop->set_key(stop, item_stop); stop->set_key(stop, item_stop);
rc = stop->search(stop); rc = stop->search(stop);
if (rc != 0) { if (rc != 0) {
ASYNC_NIF_REPLY(__strerror_term(env, rc));
start->close(start); start->close(start);
stop->close(stop); stop->close(stop);
session->close(session, NULL); session->close(session, NULL);
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(__strerror_term(env, rc));
return; return;
} }
} }
@ -893,9 +905,9 @@ ASYNC_NIF_DECL(
start and stop cursors which were opened referencing that URI. */ start and stop cursors which were opened referencing that URI. */
rc = session->truncate(session, NULL, start, stop, (const char*)config.data); rc = session->truncate(session, NULL, start, stop, (const char*)config.data);
if (start) start->close(start); start->close(start);
if (stop) stop->close(stop); stop->close(stop);
if (session) session->close(session, NULL); session->close(session, NULL);
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(rc == 0 ? ATOM_OK : __strerror_term(env, rc)); ASYNC_NIF_REPLY(rc == 0 ? ATOM_OK : __strerror_term(env, rc));
}, },
@ -938,8 +950,8 @@ ASYNC_NIF_DECL(
ErlNifBinary config; ErlNifBinary config;
if (!enif_inspect_binary(env, args->config, &config)) { if (!enif_inspect_binary(env, args->config, &config)) {
ASYNC_NIF_REPLY(enif_make_badarg(env));
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(enif_make_badarg(env));
return; return;
} }
@ -950,8 +962,8 @@ ASYNC_NIF_DECL(
WT_SESSION *session = NULL; WT_SESSION *session = NULL;
int rc = conn->open_session(conn, NULL, args->conn_handle->session_config, &session); int rc = conn->open_session(conn, NULL, args->conn_handle->session_config, &session);
if (rc != 0) { if (rc != 0) {
ASYNC_NIF_REPLY(__strerror_term(env, rc));
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(__strerror_term(env, rc));
return; return;
} }
@ -1000,8 +1012,8 @@ ASYNC_NIF_DECL(
ErlNifBinary config; ErlNifBinary config;
if (!enif_inspect_binary(env, args->config, &config)) { if (!enif_inspect_binary(env, args->config, &config)) {
ASYNC_NIF_REPLY(enif_make_badarg(env));
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(enif_make_badarg(env));
return; return;
} }
@ -1012,8 +1024,8 @@ ASYNC_NIF_DECL(
WT_SESSION *session = NULL; WT_SESSION *session = NULL;
int rc = conn->open_session(conn, NULL, args->conn_handle->session_config, &session); int rc = conn->open_session(conn, NULL, args->conn_handle->session_config, &session);
if (rc != 0) { if (rc != 0) {
ASYNC_NIF_REPLY(__strerror_term(env, rc));
enif_mutex_unlock(args->conn_handle->contexts_mutex); enif_mutex_unlock(args->conn_handle->contexts_mutex);
ASYNC_NIF_REPLY(__strerror_term(env, rc));
return; return;
} }
@ -1211,8 +1223,8 @@ ASYNC_NIF_DECL(
item_value.size = value.size; item_value.size = value.size;
cursor->set_value(cursor, &item_value); cursor->set_value(cursor, &item_value);
rc = cursor->insert(cursor); rc = cursor->insert(cursor);
ASYNC_NIF_REPLY(rc == 0 ? ATOM_OK : __strerror_term(env, rc));
__release_cursor(args->conn_handle, worker_id, args->uri, cursor); __release_cursor(args->conn_handle, worker_id, args->uri, cursor);
ASYNC_NIF_REPLY(rc == 0 ? ATOM_OK : __strerror_term(env, rc));
}, },
{ // post { // post
@ -1586,15 +1598,16 @@ ASYNC_NIF_DECL(
ASYNC_NIF_REPLY(enif_make_badarg(env)); ASYNC_NIF_REPLY(enif_make_badarg(env));
return; return;
} }
WT_ITEM item_key;
WT_ITEM item_key;
item_key.data = key.data; item_key.data = key.data;
item_key.size = key.size; item_key.size = key.size;
cursor->set_key(cursor, &item_key); cursor->set_key(cursor, &item_key);
ASYNC_NIF_REPLY(__cursor_value_ret(env, cursor, cursor->search(cursor))); ERL_NIF_TERM reply = __cursor_value_ret(env, cursor, cursor->search(cursor));
if (!args->scanning) if (!args->scanning)
(void)cursor->reset(cursor); (void)cursor->reset(cursor);
ASYNC_NIF_REPLY(reply);
}, },
{ // post { // post
@ -1649,7 +1662,15 @@ ASYNC_NIF_DECL(
cursor->set_key(cursor, &item_key); cursor->set_key(cursor, &item_key);
int rc = cursor->search_near(cursor, &exact); int rc = cursor->search_near(cursor, &exact);
if (rc == 0) { if (rc != 0) {
(void)cursor->reset(cursor);
ASYNC_NIF_REPLY(__strerror_term(env, rc));
return;
}
if (!args->scanning)
(void)cursor->reset(cursor);
if (exact == 0) { if (exact == 0) {
/* an exact match */ /* an exact match */
ASYNC_NIF_REPLY(enif_make_tuple2(env, ATOM_OK, enif_make_atom(env, "match"))); ASYNC_NIF_REPLY(enif_make_tuple2(env, ATOM_OK, enif_make_atom(env, "match")));
@ -1660,11 +1681,6 @@ ASYNC_NIF_DECL(
/* cursor now positioned at the next larger key */ /* cursor now positioned at the next larger key */
ASYNC_NIF_REPLY(enif_make_tuple2(env, ATOM_OK, enif_make_atom(env, "gt"))); ASYNC_NIF_REPLY(enif_make_tuple2(env, ATOM_OK, enif_make_atom(env, "gt")));
} }
} else {
ASYNC_NIF_REPLY(__strerror_term(env, rc));
}
if (!args->scanning)
(void)cursor->reset(cursor);
}, },
{ // post { // post
@ -1926,25 +1942,43 @@ on_unload(ErlNifEnv *env, void *priv_data)
unsigned int i; unsigned int i;
struct wterl_priv_data *priv = (struct wterl_priv_data *)priv_data; struct wterl_priv_data *priv = (struct wterl_priv_data *)priv_data;
khash_t(conns) *h; khash_t(conns) *h;
khiter_t itr; khiter_t itr_conns;
WterlConnHandle *conn_handle;
enif_mutex_lock(priv->conns_mutex); enif_mutex_lock(priv->conns_mutex);
h = priv->conns; h = priv->conns;
for (itr = kh_begin(h); itr != kh_end(h); ++itr) { for (itr_conns = kh_begin(h); itr_conns != kh_end(h); ++itr_conns) {
if (kh_exist(h, itr)) { if (kh_exist(h, itr_conns)) {
WterlConnHandle *c = kh_val(h, itr); conn_handle = kh_val(h, itr_conns);
if (c) { if (conn_handle) {
enif_mutex_lock(c->contexts_mutex); enif_mutex_lock(conn_handle->contexts_mutex);
enif_free((void*)c->session_config); enif_free((void*)conn_handle->session_config);
for (i = 0; i < ASYNC_NIF_MAX_WORKERS; i++) { for (i = 0; i < ASYNC_NIF_MAX_WORKERS; i++) {
// TODO: free keys WterlCtx *ctx = &conn_handle->contexts[i];
kh_destroy(cursors, c->contexts[i].cursors); if (ctx->session != NULL) {
WT_SESSION *session = ctx->session;
khash_t(cursors) *h = ctx->cursors;
khiter_t itr_cursors;
for (itr_cursors = kh_begin(h); itr_cursors != kh_end(h); ++itr_cursors) {
if (kh_exist(h, itr_cursors)) {
WT_CURSOR *cursor = kh_val(h, itr_cursors);
char *key = (char *)kh_key(h, itr_cursors);
cursor->close(cursor);
kh_del(cursors, h, itr_cursors);
enif_free(key);
kh_value(h, itr_cursors) = NULL;
}
}
kh_destroy(cursors, h);
session->close(session, NULL);
}
} }
} }
/* This should close all cursors and sessions. */ /* This would have closed all cursors and sessions for us
c->conn->close(c->conn, NULL); but we do that explicitly above. */
conn_handle->conn->close(conn_handle->conn, NULL);
} }
} }
@ -1952,12 +1986,12 @@ on_unload(ErlNifEnv *env, void *priv_data)
to prevent new work from coming in while shutting down. */ to prevent new work from coming in while shutting down. */
ASYNC_NIF_UNLOAD(wterl, env, priv->async_nif_priv); ASYNC_NIF_UNLOAD(wterl, env, priv->async_nif_priv);
for (itr = kh_begin(h); itr != kh_end(h); ++itr) { for (itr_conns = kh_begin(h); itr_conns != kh_end(h); ++itr_conns) {
if (kh_exist(h, itr)) { if (kh_exist(h, itr_conns)) {
WterlConnHandle *c = kh_val(h, itr); conn_handle = kh_val(h, itr_conns);
if (c) { if (conn_handle) {
enif_mutex_unlock(c->contexts_mutex); enif_mutex_unlock(conn_handle->contexts_mutex);
enif_mutex_destroy(c->contexts_mutex); enif_mutex_destroy(conn_handle->contexts_mutex);
} }
} }
} }