Skip to content

Avoid GC run between mysql_stmt_execute and mysql_stmt_store_result #705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 17, 2016
Merged

Avoid GC run between mysql_stmt_execute and mysql_stmt_store_result #705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c07feb8
Fix `MARK_CONN_INACTIVE`
kamipo Nov 29, 2015
edf08ac
`MARK_CONN_INACTIVE` is not needed
kamipo Feb 16, 2016
7cd2b28
Fix indention
kamipo Nov 29, 2015
46fa2cf
Remove needless return
kamipo Nov 29, 2015
19d93b5
Avoid GC run between `mysql_stmt_execute` and `mysql_stmt_store_result`
kamipo Nov 29, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions ext/mysql2/client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ static VALUE allocate(VALUE klass) {
mysql_client_wrapper * wrapper;
obj = Data_Make_Struct(klass, mysql_client_wrapper, rb_mysql_client_mark, rb_mysql_client_free, wrapper);
wrapper->encoding = Qnil;
MARK_CONN_INACTIVE(self);
wrapper->active_thread = Qnil;
wrapper->automatic_close = 1;
wrapper->server_version = 0;
wrapper->reconnect_enabled = 0;
Expand Down Expand Up @@ -373,7 +373,7 @@ static VALUE rb_connect(VALUE self, VALUE user, VALUE pass, VALUE host, VALUE po
if (wrapper->connect_timeout)
mysql_options(wrapper->client, MYSQL_OPT_CONNECT_TIMEOUT, &wrapper->connect_timeout);
if (rv == Qfalse)
return rb_raise_mysql2_error(wrapper);
rb_raise_mysql2_error(wrapper);
}

wrapper->server_version = mysql_get_server_version(wrapper->client);
Expand Down Expand Up @@ -418,8 +418,8 @@ static VALUE do_send_query(void *args) {
mysql_client_wrapper *wrapper = query_args->wrapper;
if ((VALUE)rb_thread_call_without_gvl(nogvl_send_query, args, RUBY_UBF_IO, 0) == Qfalse) {
/* an error occurred, we're not active anymore */
MARK_CONN_INACTIVE(self);
return rb_raise_mysql2_error(wrapper);
wrapper->active_thread = Qnil;
rb_raise_mysql2_error(wrapper);
}
return Qnil;
}
Expand Down Expand Up @@ -448,7 +448,7 @@ static void *nogvl_do_result(void *ptr, char use_result) {

/* once our result is stored off, this connection is
ready for another command to be issued */
MARK_CONN_INACTIVE(self);
wrapper->active_thread = Qnil;

return result;
}
Expand Down Expand Up @@ -480,8 +480,8 @@ static VALUE rb_mysql_client_async_result(VALUE self) {
REQUIRE_CONNECTED(wrapper);
if ((VALUE)rb_thread_call_without_gvl(nogvl_read_query_result, wrapper->client, RUBY_UBF_IO, 0) == Qfalse) {
/* an error occurred, mark this connection inactive */
MARK_CONN_INACTIVE(self);
return rb_raise_mysql2_error(wrapper);
wrapper->active_thread = Qnil;
rb_raise_mysql2_error(wrapper);
}

is_streaming = rb_hash_aref(rb_iv_get(self, "@current_query_options"), sym_stream);
Expand All @@ -493,7 +493,7 @@ static VALUE rb_mysql_client_async_result(VALUE self) {

if (result == NULL) {
if (mysql_errno(wrapper->client) != 0) {
MARK_CONN_INACTIVE(self);
wrapper->active_thread = Qnil;
rb_raise_mysql2_error(wrapper);
}
/* no data and no error, so query was not a SELECT */
Expand All @@ -517,7 +517,7 @@ struct async_query_args {
static VALUE disconnect_and_raise(VALUE self, VALUE error) {
GET_CLIENT(self);

MARK_CONN_INACTIVE(self);
wrapper->active_thread = Qnil;
wrapper->connected = 0;

/* Invalidate the MySQL socket to prevent further communication.
Expand Down Expand Up @@ -588,7 +588,7 @@ static VALUE finish_and_mark_inactive(void *args) {
result = (MYSQL_RES *)rb_thread_call_without_gvl(nogvl_store_result, wrapper, RUBY_UBF_IO, 0);
mysql_free_result(result);

MARK_CONN_INACTIVE(self);
wrapper->active_thread = Qnil;
}

return Qnil;
Expand Down Expand Up @@ -1011,10 +1011,10 @@ static VALUE rb_mysql_client_ping(VALUE self) {
static VALUE rb_mysql_client_more_results(VALUE self)
{
GET_CLIENT(self);
if (mysql_more_results(wrapper->client) == 0)
return Qfalse;
else
return Qtrue;
if (mysql_more_results(wrapper->client) == 0)
return Qfalse;
else
return Qtrue;
}

/* call-seq:
Expand Down Expand Up @@ -1228,7 +1228,7 @@ static VALUE initialize_ext(VALUE self) {

if ((VALUE)rb_thread_call_without_gvl(nogvl_init, wrapper, RUBY_UBF_IO, 0) == Qfalse) {
/* TODO: warning - not enough memory? */
return rb_raise_mysql2_error(wrapper);
rb_raise_mysql2_error(wrapper);
}

wrapper->initialized = 1;
Expand Down
4 changes: 0 additions & 4 deletions ext/mysql2/client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,6 @@ typedef struct {

void rb_mysql_client_set_active_thread(VALUE self);

#define MARK_CONN_INACTIVE(conn) do {\
wrapper->active_thread = Qnil; \
} while(0)

#define GET_CLIENT(self) \
mysql_client_wrapper *wrapper; \
Data_Get_Struct(self, mysql_client_wrapper, wrapper);
Expand Down
20 changes: 3 additions & 17 deletions ext/mysql2/statement.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ static VALUE intern_usec, intern_sec, intern_min, intern_hour, intern_day, inter
if (!stmt_wrapper->stmt) { rb_raise(cMysql2Error, "Invalid statement handle"); } \
if (stmt_wrapper->closed) { rb_raise(cMysql2Error, "Statement handle already closed"); }


static void rb_mysql_stmt_mark(void * ptr) {
mysql_stmt_wrapper *stmt_wrapper = ptr;
if (!stmt_wrapper) return;
Expand Down Expand Up @@ -42,7 +41,6 @@ void decr_mysql2_stmt(mysql_stmt_wrapper *stmt_wrapper) {
}
}


void rb_raise_mysql2_stmt_error(mysql_stmt_wrapper *stmt_wrapper) {
VALUE e;
GET_CLIENT(stmt_wrapper->client);
Expand Down Expand Up @@ -71,7 +69,6 @@ void rb_raise_mysql2_stmt_error(mysql_stmt_wrapper *stmt_wrapper) {
rb_exc_raise(e);
}


/*
* used to pass all arguments to mysql_stmt_prepare while inside
* nogvl_prepare_statement_args
Expand Down Expand Up @@ -180,16 +177,6 @@ static void *nogvl_execute(void *ptr) {
}
}

static void *nogvl_stmt_store_result(void *ptr) {
MYSQL_STMT *stmt = ptr;

if (mysql_stmt_store_result(stmt)) {
return (void *)Qfalse;
} else {
return (void *)Qtrue;
}
}

static void set_buffer_for_string(MYSQL_BIND* bind_buffer, unsigned long *length_buffer, VALUE string) {
int length;

Expand Down Expand Up @@ -369,8 +356,7 @@ static VALUE execute(int argc, VALUE *argv, VALUE self) {
if (metadata == NULL) {
if (mysql_stmt_errno(stmt) != 0) {
// either CR_OUT_OF_MEMORY or CR_UNKNOWN_ERROR. both fatal.

MARK_CONN_INACTIVE(stmt_wrapper->client);
wrapper->active_thread = Qnil;
rb_raise_mysql2_stmt_error(stmt_wrapper);
}
// no data and no error, so query was not a SELECT
Expand All @@ -384,11 +370,11 @@ static VALUE execute(int argc, VALUE *argv, VALUE self) {
is_streaming = (Qtrue == rb_hash_aref(current, sym_stream));
if (!is_streaming) {
// recieve the whole result set from the server
if (rb_thread_call_without_gvl(nogvl_stmt_store_result, stmt, RUBY_UBF_IO, 0) == Qfalse) {
if (mysql_stmt_store_result(stmt)) {
mysql_free_result(metadata);
rb_raise_mysql2_stmt_error(stmt_wrapper);
}
MARK_CONN_INACTIVE(stmt_wrapper->client);
wrapper->active_thread = Qnil;
}

resultObj = rb_mysql_result_to_obj(stmt_wrapper->client, wrapper->encoding, current, metadata, self);
Expand Down