Skip to content

Fixed error handling to prevent crashing ruby in 0.6.x #133

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 2 commits into from
Aug 31, 2013
Merged

Fixed error handling to prevent crashing ruby in 0.6.x #133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7af16cc
Added test that causes segfault
wbond Aug 5, 2013
3eb33c6
Fixed error handling to prevent crashing ruby
wbond Aug 20, 2013
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
50 changes: 46 additions & 4 deletions ext/tiny_tds/client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ VALUE opt_escape_regex, opt_escape_dblquote;

// Lib Backend (Helpers)

static VALUE rb_tinytds_raise_error(DBPROCESS *dbproc, int cancel, char *error, char *source, int severity, int dberr, int oserr) {
VALUE rb_tinytds_raise_error(DBPROCESS *dbproc, int cancel, char *error, char *source, int severity, int dberr, int oserr) {
GET_CLIENT_USERDATA(dbproc);
if (cancel && !dbdead(dbproc) && userdata && !userdata->closed) {
userdata->dbsqlok_sent = 1;
Expand Down Expand Up @@ -92,14 +92,54 @@ int tinytds_err_handler(DBPROCESS *dbproc, int severity, int dberr, int oserr, c
break;
}
}
rb_tinytds_raise_error(dbproc, cancel, dberrstr, source, severity, dberr, oserr);
/*
When in non-blocking mode we need to store the exception data to throw it
once the blocking call returns, otherwise we will segfault ruby since part
of the contract of the ruby non-blocking indicator is that you do not call
any of the ruby C API.
*/
if (userdata && userdata->nonblocking) {
/*
If we've already captured an error message, don't overwrite it. This is
here because FreeTDS sends a generic "General SQL Server error" message
that will overwrite the real message. This is not normally a problem
because a ruby exception is normally thrown and we bail before the
generic message can be sent.
*/
if (!userdata->nonblocking_error.is_set) {
userdata->nonblocking_error.cancel = cancel;
strcpy(userdata->nonblocking_error.error, dberrstr);
strcpy(userdata->nonblocking_error.source, source);
userdata->nonblocking_error.severity = severity;
userdata->nonblocking_error.dberr = dberr;
userdata->nonblocking_error.oserr = oserr;
userdata->nonblocking_error.is_set = 1;
}
} else {
rb_tinytds_raise_error(dbproc, cancel, dberrstr, source, severity, dberr, oserr);
}
return return_value;
}

int tinytds_msg_handler(DBPROCESS *dbproc, DBINT msgno, int msgstate, int severity, char *msgtext, char *srvname, char *procname, int line) {
static char *source = "message";
if (severity > 10)
rb_tinytds_raise_error(dbproc, 1, msgtext, source, severity, msgno, msgstate);
GET_CLIENT_USERDATA(dbproc);
if (severity > 10) {
// See tinytds_err_handler() for info about why we do this
if (userdata && userdata->nonblocking) {
if (!userdata->nonblocking_error.is_set) {
userdata->nonblocking_error.cancel = 1;
strcpy(userdata->nonblocking_error.error, msgtext);
strcpy(userdata->nonblocking_error.source, source);
userdata->nonblocking_error.severity = severity;
userdata->nonblocking_error.dberr = msgno;
userdata->nonblocking_error.oserr = msgstate;
userdata->nonblocking_error.is_set = 1;
}
} else {
rb_tinytds_raise_error(dbproc, 1, msgtext, source, severity, msgno, msgstate);
}
}
return 0;
}

Expand All @@ -108,6 +148,8 @@ static void rb_tinytds_client_reset_userdata(tinytds_client_userdata *userdata)
userdata->dbsql_sent = 0;
userdata->dbsqlok_sent = 0;
userdata->dbcancel_sent = 0;
userdata->nonblocking = 0;
userdata->nonblocking_error.is_set = 0;
}

static void rb_tinytds_client_mark(void *ptr) {
Expand Down
12 changes: 12 additions & 0 deletions ext/tiny_tds/client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,25 @@

void init_tinytds_client();

typedef struct {
short int is_set;
int cancel;
char error[1024];
char source[1024];
int severity;
int dberr;
int oserr;
} tinytds_errordata;

typedef struct {
short int closed;
short int timing_out;
short int dbsql_sent;
short int dbsqlok_sent;
RETCODE dbsqlok_retcode;
short int dbcancel_sent;
short int nonblocking;
tinytds_errordata nonblocking_error;
} tinytds_client_userdata;

typedef struct {
Expand Down
76 changes: 57 additions & 19 deletions ext/tiny_tds/result.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,35 +79,73 @@ VALUE rb_tinytds_new_result_obj(tinytds_client_wrapper *cwrap) {

#define NOGVL_DBCALL(_dbfunction, _client) ( \
(RETCODE)rb_thread_blocking_region( \
(rb_blocking_function_t*)nogvl_ ## _dbfunction, _client, \
(rb_blocking_function_t*)_dbfunction, _client, \
(rb_unblock_function_t*)dbcancel_ubf, _client ) \
)

static void dbcancel_ubf(DBPROCESS *client) {
GET_CLIENT_USERDATA(client);
dbcancel(client);
userdata->dbcancel_sent = 1;
userdata->dbsql_sent = 0;
}

static void nogvl_setup(DBPROCESS *client) {
GET_CLIENT_USERDATA(client);
userdata->nonblocking = 1;
}

static void nogvl_cleanup(DBPROCESS *client) {
GET_CLIENT_USERDATA(client);
userdata->nonblocking = 0;
/*
Now that the blocking operation is done, we can finally throw any
exceptions based on errors from SQL Server.
*/
if (userdata->nonblocking_error.is_set) {
userdata->nonblocking_error.is_set = 0;
rb_tinytds_raise_error(client,
userdata->nonblocking_error.cancel,
&userdata->nonblocking_error.error,
&userdata->nonblocking_error.source,
userdata->nonblocking_error.severity,
userdata->nonblocking_error.dberr,
userdata->nonblocking_error.oserr);
}
}

static RETCODE nogvl_dbsqlok(DBPROCESS *client) {
int retcode = FAIL;
GET_CLIENT_USERDATA(client);
retcode = dbsqlok(client);
nogvl_setup(client);
retcode = NOGVL_DBCALL(dbsqlok, client);
nogvl_cleanup(client);
userdata->dbsqlok_sent = 1;
return retcode;
}

static RETCODE nogvl_dbsqlexec(DBPROCESS *client) {
return dbsqlexec(client);
int retcode = FAIL;
nogvl_setup(client);
retcode = NOGVL_DBCALL(dbsqlexec, client);
nogvl_cleanup(client);
return retcode;
}

static RETCODE nogvl_dbresults(DBPROCESS *client) {
return dbresults(client);
int retcode = FAIL;
nogvl_setup(client);
retcode = NOGVL_DBCALL(dbresults, client);
nogvl_cleanup(client);
return retcode;
}

static RETCODE nogvl_dbnextrow(DBPROCESS * client) {
return dbnextrow(client);
}

static void dbcancel_ubf(DBPROCESS *client) {
GET_CLIENT_USERDATA(client);
dbcancel(client);
userdata->dbcancel_sent = 1;
userdata->dbsql_sent = 0;
int retcode = FAIL;
nogvl_setup(client);
retcode = NOGVL_DBCALL(dbnextrow, client);
nogvl_cleanup(client);
return retcode;
}

// Lib Backend (Helpers)
Expand All @@ -118,7 +156,7 @@ static RETCODE rb_tinytds_result_dbresults_retcode(VALUE self) {
RETCODE db_rc;
ruby_rc = rb_ary_entry(rwrap->dbresults_retcodes, rwrap->number_of_results);
if (NIL_P(ruby_rc)) {
db_rc = NOGVL_DBCALL(dbresults, rwrap->client);
db_rc = nogvl_dbresults(rwrap->client);
ruby_rc = INT2FIX(db_rc);
rb_ary_store(rwrap->dbresults_retcodes, rwrap->number_of_results, ruby_rc);
} else {
Expand All @@ -130,7 +168,7 @@ static RETCODE rb_tinytds_result_dbresults_retcode(VALUE self) {
static RETCODE rb_tinytds_result_ok_helper(DBPROCESS *client) {
GET_CLIENT_USERDATA(client);
if (userdata->dbsqlok_sent == 0) {
userdata->dbsqlok_retcode = NOGVL_DBCALL(dbsqlok, client);
userdata->dbsqlok_retcode = nogvl_dbsqlok(client);
}
return userdata->dbsqlok_retcode;
}
Expand Down Expand Up @@ -373,7 +411,7 @@ static VALUE rb_tinytds_result_each(int argc, VALUE * argv, VALUE self) {
/* Create rows for this result set. */
unsigned long rowi = 0;
VALUE result = rb_ary_new();
while (NOGVL_DBCALL(dbnextrow, rwrap->client) != NO_MORE_ROWS) {
while (nogvl_dbnextrow(rwrap->client) != NO_MORE_ROWS) {
VALUE row = rb_tinytds_result_fetch_row(self, timezone, symbolize_keys, as_array);
if (cache_rows)
rb_ary_store(result, rowi, row);
Expand Down Expand Up @@ -406,7 +444,7 @@ static VALUE rb_tinytds_result_each(int argc, VALUE * argv, VALUE self) {
} else {
// If we do not find results, side step the rb_tinytds_result_dbresults_retcode helper and
// manually populate its memoized array while nullifing any memoized fields too before loop.
dbresults_rc = NOGVL_DBCALL(dbresults, rwrap->client);
dbresults_rc = nogvl_dbresults(rwrap->client);
rb_ary_store(rwrap->dbresults_retcodes, rwrap->number_of_results, INT2FIX(dbresults_rc));
rb_ary_store(rwrap->fields_processed, rwrap->number_of_results, Qnil);
}
Expand Down Expand Up @@ -466,10 +504,10 @@ static VALUE rb_tinytds_result_insert(VALUE self) {
rb_tinytds_result_cancel_helper(rwrap->client);
VALUE identity = Qnil;
dbcmd(rwrap->client, rwrap->cwrap->identity_insert_sql);
if (NOGVL_DBCALL(dbsqlexec, rwrap->client) != FAIL
&& NOGVL_DBCALL(dbresults, rwrap->client) != FAIL
if (nogvl_dbsqlexec(rwrap->client) != FAIL
&& nogvl_dbresults(rwrap->client) != FAIL
&& DBROWS(rwrap->client) != FAIL) {
while (NOGVL_DBCALL(dbnextrow, rwrap->client) != NO_MORE_ROWS) {
while (nogvl_dbnextrow(rwrap->client) != NO_MORE_ROWS) {
int col = 1;
BYTE *data = dbdata(rwrap->client, col);
DBINT data_len = dbdatlen(rwrap->client, col);
Expand Down
20 changes: 20 additions & 0 deletions test/thread_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,26 @@ class ThreadTest < TinyTds::TestCase
assert x > mintime, "#{x} is not slower than #{mintime} seconds"
end

it 'should not crash on error in parallel' do
threads = []
@numthreads.times do |i|
start = Time.new
threads << Thread.new do
@pool.with do |client|
begin
result = client.execute "select dbname()"
result.each { |r| puts r }
rescue Exception => e
# We are throwing an error on purpose here since 0.6.1 would
# segfault on errors thrown in threads
end
end
end
end
threads.each { |t| t.join }
assert true
end

end

end
Expand Down