Skip to content

Commit 4473f7a

Browse files
metaskillsmetaskills
committed
Merge pull request #133 from veracross/thread_crash_on_error
Fixed error handling to prevent crashing ruby in 0.6.x
2 parents 7f9d88c + 3eb33c6 commit 4473f7a

File tree

4 files changed

+135
-23
lines changed

4 files changed

+135
-23
lines changed

‎ext/tiny_tds/client.c

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ VALUE opt_escape_regex, opt_escape_dblquote;
2424

2525
// Lib Backend (Helpers)
2626

27-
static VALUE rb_tinytds_raise_error(DBPROCESS *dbproc, int cancel, char *error, char *source, int severity, int dberr, int oserr) {
27+
VALUE rb_tinytds_raise_error(DBPROCESS *dbproc, int cancel, char *error, char *source, int severity, int dberr, int oserr) {
2828
GET_CLIENT_USERDATA(dbproc);
2929
if (cancel && !dbdead(dbproc) && userdata && !userdata->closed) {
3030
userdata->dbsqlok_sent = 1;
@@ -92,14 +92,54 @@ int tinytds_err_handler(DBPROCESS *dbproc, int severity, int dberr, int oserr, c
9292
break;
9393
}
9494
}
95-
rb_tinytds_raise_error(dbproc, cancel, dberrstr, source, severity, dberr, oserr);
95+
/*
96+
When in non-blocking mode we need to store the exception data to throw it
97+
once the blocking call returns, otherwise we will segfault ruby since part
98+
of the contract of the ruby non-blocking indicator is that you do not call
99+
any of the ruby C API.
100+
*/
101+
if (userdata && userdata->nonblocking) {
102+
/*
103+
If we've already captured an error message, don't overwrite it. This is
104+
here because FreeTDS sends a generic "General SQL Server error" message
105+
that will overwrite the real message. This is not normally a problem
106+
because a ruby exception is normally thrown and we bail before the
107+
generic message can be sent.
108+
*/
109+
if (!userdata->nonblocking_error.is_set) {
110+
userdata->nonblocking_error.cancel = cancel;
111+
strcpy(userdata->nonblocking_error.error, dberrstr);
112+
strcpy(userdata->nonblocking_error.source, source);
113+
userdata->nonblocking_error.severity = severity;
114+
userdata->nonblocking_error.dberr = dberr;
115+
userdata->nonblocking_error.oserr = oserr;
116+
userdata->nonblocking_error.is_set = 1;
117+
}
118+
} else {
119+
rb_tinytds_raise_error(dbproc, cancel, dberrstr, source, severity, dberr, oserr);
120+
}
96121
return return_value;
97122
}
98123

99124
int tinytds_msg_handler(DBPROCESS *dbproc, DBINT msgno, int msgstate, int severity, char *msgtext, char *srvname, char *procname, int line) {
100125
static char *source = "message";
101-
if (severity > 10)
102-
rb_tinytds_raise_error(dbproc, 1, msgtext, source, severity, msgno, msgstate);
126+
GET_CLIENT_USERDATA(dbproc);
127+
if (severity > 10) {
128+
// See tinytds_err_handler() for info about why we do this
129+
if (userdata && userdata->nonblocking) {
130+
if (!userdata->nonblocking_error.is_set) {
131+
userdata->nonblocking_error.cancel = 1;
132+
strcpy(userdata->nonblocking_error.error, msgtext);
133+
strcpy(userdata->nonblocking_error.source, source);
134+
userdata->nonblocking_error.severity = severity;
135+
userdata->nonblocking_error.dberr = msgno;
136+
userdata->nonblocking_error.oserr = msgstate;
137+
userdata->nonblocking_error.is_set = 1;
138+
}
139+
} else {
140+
rb_tinytds_raise_error(dbproc, 1, msgtext, source, severity, msgno, msgstate);
141+
}
142+
}
103143
return 0;
104144
}
105145

@@ -108,6 +148,8 @@ static void rb_tinytds_client_reset_userdata(tinytds_client_userdata *userdata)
108148
userdata->dbsql_sent = 0;
109149
userdata->dbsqlok_sent = 0;
110150
userdata->dbcancel_sent = 0;
151+
userdata->nonblocking = 0;
152+
userdata->nonblocking_error.is_set = 0;
111153
}
112154

113155
static void rb_tinytds_client_mark(void *ptr) {

‎ext/tiny_tds/client.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,25 @@
44

55
void init_tinytds_client();
66

7+
typedef struct {
8+
short int is_set;
9+
int cancel;
10+
char error[1024];
11+
char source[1024];
12+
int severity;
13+
int dberr;
14+
int oserr;
15+
} tinytds_errordata;
16+
717
typedef struct {
818
short int closed;
919
short int timing_out;
1020
short int dbsql_sent;
1121
short int dbsqlok_sent;
1222
RETCODE dbsqlok_retcode;
1323
short int dbcancel_sent;
24+
short int nonblocking;
25+
tinytds_errordata nonblocking_error;
1426
} tinytds_client_userdata;
1527

1628
typedef struct {

‎ext/tiny_tds/result.c

Lines changed: 57 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -79,35 +79,73 @@ VALUE rb_tinytds_new_result_obj(tinytds_client_wrapper *cwrap) {
7979

8080
#define NOGVL_DBCALL(_dbfunction, _client) ( \
8181
(RETCODE)rb_thread_blocking_region( \
82-
(rb_blocking_function_t*)nogvl_ ## _dbfunction, _client, \
82+
(rb_blocking_function_t*)_dbfunction, _client, \
8383
(rb_unblock_function_t*)dbcancel_ubf, _client ) \
8484
)
8585

86+
static void dbcancel_ubf(DBPROCESS *client) {
87+
GET_CLIENT_USERDATA(client);
88+
dbcancel(client);
89+
userdata->dbcancel_sent = 1;
90+
userdata->dbsql_sent = 0;
91+
}
92+
93+
static void nogvl_setup(DBPROCESS *client) {
94+
GET_CLIENT_USERDATA(client);
95+
userdata->nonblocking = 1;
96+
}
97+
98+
static void nogvl_cleanup(DBPROCESS *client) {
99+
GET_CLIENT_USERDATA(client);
100+
userdata->nonblocking = 0;
101+
/*
102+
Now that the blocking operation is done, we can finally throw any
103+
exceptions based on errors from SQL Server.
104+
*/
105+
if (userdata->nonblocking_error.is_set) {
106+
userdata->nonblocking_error.is_set = 0;
107+
rb_tinytds_raise_error(client,
108+
userdata->nonblocking_error.cancel,
109+
&userdata->nonblocking_error.error,
110+
&userdata->nonblocking_error.source,
111+
userdata->nonblocking_error.severity,
112+
userdata->nonblocking_error.dberr,
113+
userdata->nonblocking_error.oserr);
114+
}
115+
}
116+
86117
static RETCODE nogvl_dbsqlok(DBPROCESS *client) {
87118
int retcode = FAIL;
88119
GET_CLIENT_USERDATA(client);
89-
retcode = dbsqlok(client);
120+
nogvl_setup(client);
121+
retcode = NOGVL_DBCALL(dbsqlok, client);
122+
nogvl_cleanup(client);
90123
userdata->dbsqlok_sent = 1;
91124
return retcode;
92125
}
93126

94127
static RETCODE nogvl_dbsqlexec(DBPROCESS *client) {
95-
return dbsqlexec(client);
128+
int retcode = FAIL;
129+
nogvl_setup(client);
130+
retcode = NOGVL_DBCALL(dbsqlexec, client);
131+
nogvl_cleanup(client);
132+
return retcode;
96133
}
97134

98135
static RETCODE nogvl_dbresults(DBPROCESS *client) {
99-
return dbresults(client);
136+
int retcode = FAIL;
137+
nogvl_setup(client);
138+
retcode = NOGVL_DBCALL(dbresults, client);
139+
nogvl_cleanup(client);
140+
return retcode;
100141
}
101142

102143
static RETCODE nogvl_dbnextrow(DBPROCESS * client) {
103-
return dbnextrow(client);
104-
}
105-
106-
static void dbcancel_ubf(DBPROCESS *client) {
107-
GET_CLIENT_USERDATA(client);
108-
dbcancel(client);
109-
userdata->dbcancel_sent = 1;
110-
userdata->dbsql_sent = 0;
144+
int retcode = FAIL;
145+
nogvl_setup(client);
146+
retcode = NOGVL_DBCALL(dbnextrow, client);
147+
nogvl_cleanup(client);
148+
return retcode;
111149
}
112150

113151
// Lib Backend (Helpers)
@@ -118,7 +156,7 @@ static RETCODE rb_tinytds_result_dbresults_retcode(VALUE self) {
118156
RETCODE db_rc;
119157
ruby_rc = rb_ary_entry(rwrap->dbresults_retcodes, rwrap->number_of_results);
120158
if (NIL_P(ruby_rc)) {
121-
db_rc = NOGVL_DBCALL(dbresults, rwrap->client);
159+
db_rc = nogvl_dbresults(rwrap->client);
122160
ruby_rc = INT2FIX(db_rc);
123161
rb_ary_store(rwrap->dbresults_retcodes, rwrap->number_of_results, ruby_rc);
124162
} else {
@@ -130,7 +168,7 @@ static RETCODE rb_tinytds_result_dbresults_retcode(VALUE self) {
130168
static RETCODE rb_tinytds_result_ok_helper(DBPROCESS *client) {
131169
GET_CLIENT_USERDATA(client);
132170
if (userdata->dbsqlok_sent == 0) {
133-
userdata->dbsqlok_retcode = NOGVL_DBCALL(dbsqlok, client);
171+
userdata->dbsqlok_retcode = nogvl_dbsqlok(client);
134172
}
135173
return userdata->dbsqlok_retcode;
136174
}
@@ -373,7 +411,7 @@ static VALUE rb_tinytds_result_each(int argc, VALUE * argv, VALUE self) {
373411
/* Create rows for this result set. */
374412
unsigned long rowi = 0;
375413
VALUE result = rb_ary_new();
376-
while (NOGVL_DBCALL(dbnextrow, rwrap->client) != NO_MORE_ROWS) {
414+
while (nogvl_dbnextrow(rwrap->client) != NO_MORE_ROWS) {
377415
VALUE row = rb_tinytds_result_fetch_row(self, timezone, symbolize_keys, as_array);
378416
if (cache_rows)
379417
rb_ary_store(result, rowi, row);
@@ -406,7 +444,7 @@ static VALUE rb_tinytds_result_each(int argc, VALUE * argv, VALUE self) {
406444
} else {
407445
// If we do not find results, side step the rb_tinytds_result_dbresults_retcode helper and
408446
// manually populate its memoized array while nullifing any memoized fields too before loop.
409-
dbresults_rc = NOGVL_DBCALL(dbresults, rwrap->client);
447+
dbresults_rc = nogvl_dbresults(rwrap->client);
410448
rb_ary_store(rwrap->dbresults_retcodes, rwrap->number_of_results, INT2FIX(dbresults_rc));
411449
rb_ary_store(rwrap->fields_processed, rwrap->number_of_results, Qnil);
412450
}
@@ -466,10 +504,10 @@ static VALUE rb_tinytds_result_insert(VALUE self) {
466504
rb_tinytds_result_cancel_helper(rwrap->client);
467505
VALUE identity = Qnil;
468506
dbcmd(rwrap->client, rwrap->cwrap->identity_insert_sql);
469-
if (NOGVL_DBCALL(dbsqlexec, rwrap->client) != FAIL
470-
&& NOGVL_DBCALL(dbresults, rwrap->client) != FAIL
507+
if (nogvl_dbsqlexec(rwrap->client) != FAIL
508+
&& nogvl_dbresults(rwrap->client) != FAIL
471509
&& DBROWS(rwrap->client) != FAIL) {
472-
while (NOGVL_DBCALL(dbnextrow, rwrap->client) != NO_MORE_ROWS) {
510+
while (nogvl_dbnextrow(rwrap->client) != NO_MORE_ROWS) {
473511
int col = 1;
474512
BYTE *data = dbdata(rwrap->client, col);
475513
DBINT data_len = dbdatlen(rwrap->client, col);

‎test/thread_test.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,26 @@ class ThreadTest < TinyTds::TestCase
3838
assert x > mintime, "#{x} is not slower than #{mintime} seconds"
3939
end
4040

41+
it 'should not crash on error in parallel' do
42+
threads = []
43+
@numthreads.times do |i|
44+
start = Time.new
45+
threads << Thread.new do
46+
@pool.with do |client|
47+
begin
48+
result = client.execute "select dbname()"
49+
result.each { |r| puts r }
50+
rescue Exception => e
51+
# We are throwing an error on purpose here since 0.6.1 would
52+
# segfault on errors thrown in threads
53+
end
54+
end
55+
end
56+
end
57+
threads.each { |t| t.join }
58+
assert true
59+
end
60+
4161
end
4262

4363
end

0 commit comments

Comments
 (0)