Skip to content

Commit 7731270

Browse files
yahondaclaude
andcommitted
Add Cursor#reparse_between_execs to work around OCI re-exec deadlock
Re-executing the same parsed cursor under CURSOR_SHARING=FORCE (or SIMILAR) when the SQL contains BOTH a literal AND a RETURNING ... INTO :bind clause hangs forever on the second exec. The server reports "SQL*Net more data from client"; the client sits in OCIStmtExecute -> kpuexec -> ... -> nttfprd -> read(). A SQL*Net wire trace shows OCIStmtExecute constructs a 31-byte fast-path "execute already-parsed cursor" packet, and that packet differs from the working (CURSOR_SHARING=EXACT) case by exactly one byte computed inside libclntsh from cursor state set by exec kubo#1's response. ruby-oci8 calls OCIStmtExecute with identical arguments in both runs, so the defect is in the Oracle Client TTC marshalling, not ruby-oci8. Add an opt-in Cursor#reparse_between_execs accessor (default false) that releases and re-prepares the underlying statement handle before every exec after the first, forcing OCI onto the full parse+exec path and avoiding the buggy fast path. Bind values are preserved. - ext/oci8/stmt.c: add private __reprepare(sql), and let oci8_bind cleanly detach a bind object from a previous statement so the same valuep/indp buffers can be re-attached to a fresh statement handle. - lib/oci8/cursor.rb: remember the parsed SQL on initialize, expose reparse_between_execs, and re-bind every entry in @bind_handles after __reprepare. - test/test_reparse_between_execs.rb: regression test covering the literal + RETURNING + same-cursor scenario plus a plain repeated INSERT to ensure the flag does not affect ordinary cursors. The flag is off by default; existing cursors keep their fast-path behavior. Leave it false unless you actually hit the deadlock - the extra round trip costs measurable latency on hot paths. Refs: rsim/oracle-enhanced#2619, rsim/oracle-enhanced#2620. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 47e82ac commit 7731270

4 files changed

Lines changed: 192 additions & 10 deletions

File tree

ext/oci8/stmt.c

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,48 @@ static VALUE oci8_stmt_initialize(VALUE self, VALUE svc, VALUE sql)
9797
return Qnil;
9898
}
9999

100+
/*
101+
* @overload __reprepare(sql_statement)
102+
*
103+
* Releases the underlying OCI statement handle and re-prepares it
104+
* with the same SQL text. Workaround for an Oracle Client TTC
105+
* fast-path bug under CURSOR_SHARING + RETURNING + same-cursor
106+
* re-execute. Caller must re-attach all bind/define handles after
107+
* this call.
108+
*
109+
* @param [String] sql_statement
110+
* @private
111+
*/
112+
static VALUE oci8_stmt_reprepare(VALUE self, VALUE sql)
113+
{
114+
oci8_stmt_t *stmt = TO_STMT(self);
115+
oci8_svcctx_t *svcctx;
116+
sword rv;
117+
118+
OCI8SafeStringValue(sql);
119+
svcctx = oci8_get_svcctx(stmt->svc);
120+
oci8_check_pid_consistency(svcctx);
121+
122+
if (stmt->use_stmt_release) {
123+
OCIStmtRelease(stmt->base.hp.stmt, oci8_errhp, NULL, 0, OCI_DEFAULT);
124+
stmt->use_stmt_release = 0;
125+
} else if (stmt->base.hp.ptr != NULL) {
126+
OCIHandleFree(stmt->base.hp.ptr, OCI_HTYPE_STMT);
127+
}
128+
stmt->base.hp.ptr = NULL;
129+
stmt->base.type = 0;
130+
131+
rv = OCIStmtPrepare2(svcctx->base.hp.svc, &stmt->base.hp.stmt, oci8_errhp,
132+
RSTRING_ORATEXT(sql), RSTRING_LENINT(sql),
133+
NULL, 0, OCI_NTV_SYNTAX, OCI_DEFAULT);
134+
if (IS_OCI_ERROR(rv)) {
135+
chker2(rv, &svcctx->base);
136+
}
137+
stmt->base.type = OCI_HTYPE_STMT;
138+
stmt->use_stmt_release = 1;
139+
return self;
140+
}
141+
100142
/*
101143
* @overload __define(position, bindobj)
102144
*
@@ -196,7 +238,15 @@ static VALUE oci8_bind(VALUE self, VALUE vplaceholder, VALUE vbindobj)
196238
}
197239
obind = TO_BIND(vbindobj); /* 2 */
198240
if (obind->base.hp.bnd != NULL) {
199-
oci8_base_free(&obind->base); /* TODO: OK? */
241+
/* The bind object is already attached to a statement. Detach
242+
* cleanly so OCIBindByPos / OCIBindByName below installs a
243+
* fresh OCI bind handle on the (possibly new) statement,
244+
* while reusing the existing valuep / indp buffers. The OCI
245+
* bind handle itself is owned by the previous statement and
246+
* is freed when that statement is released. */
247+
oci8_unlink_from_parent(&obind->base);
248+
obind->base.hp.bnd = NULL;
249+
obind->base.type = 0;
200250
}
201251
data_type = (const oci8_bind_data_type_t *)obind->base.data_type;
202252

@@ -218,6 +268,7 @@ static VALUE oci8_bind(VALUE self, VALUE vplaceholder, VALUE vbindobj)
218268
chker3(status, &stmt->base, stmt->base.hp.stmt);
219269
}
220270
obind->base.type = OCI_HTYPE_BIND;
271+
obind->base.closed = 0;
221272
/* link to the parent as soon as possible to preserve deallocation order. */
222273
oci8_unlink_from_parent((oci8_base_t*)obind);
223274
oci8_link_to_parent((oci8_base_t*)obind, (oci8_base_t*)stmt);
@@ -437,6 +488,7 @@ void Init_oci8_stmt(VALUE cOCI8)
437488
cOCIStmt = oci8_define_class_under(cOCI8, "Cursor", &oci8_stmt_data_type, oci8_stmt_alloc);
438489

439490
rb_define_private_method(cOCIStmt, "__initialize", oci8_stmt_initialize, 2);
491+
rb_define_private_method(cOCIStmt, "__reprepare", oci8_stmt_reprepare, 1);
440492
rb_define_private_method(cOCIStmt, "__define", oci8_define_by_pos, 2);
441493
rb_define_private_method(cOCIStmt, "__bind", oci8_bind, 2);
442494
rb_define_private_method(cOCIStmt, "__execute", oci8_stmt_execute, 1);

lib/oci8/cursor.rb

Lines changed: 46 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,35 @@ def initialize(conn, sql = nil)
2828
@fetch_array_size = nil
2929
@rowbuf_size = 0
3030
@rowbuf_index = 0
31+
@sql = sql
32+
@reparse_between_execs = false
33+
@already_executed = false
3134
__initialize(conn, sql) # Initialize the internal C structure.
3235
self.prefetch_rows = conn.instance_variable_get(:@prefetch_rows)
3336
end
3437

38+
# When true, releases and re-prepares the underlying OCI statement
39+
# handle before every {#exec} call after the first. Default false.
40+
#
41+
# Workaround for an Oracle Client (libclntsh) defect that produces
42+
# a half-duplex SQL*Net deadlock when ALL of the following hold:
43+
#
44+
# 1. session has +CURSOR_SHARING = FORCE+ (or +SIMILAR+),
45+
# 2. SQL contains a literal AND a +RETURNING ... INTO :bind+ clause,
46+
# 3. the same parsed cursor is executed more than once.
47+
#
48+
# Under that combination, OCIStmtExecute builds a fast-path "execute
49+
# already-parsed cursor" packet that the server treats as
50+
# incomplete; both ends then wait on the other forever. Setting
51+
# this flag forces a full parse+exec round trip on each call,
52+
# avoiding the buggy fast path. Bind values are preserved.
53+
#
54+
# Leave this off unless you actually hit the deadlock — the extra
55+
# round trip costs measurable latency on hot paths.
56+
#
57+
# @return [Boolean]
58+
attr_accessor :reparse_between_execs
59+
3560
# explicitly indicate the date type of fetched value. run this
3661
# method within parse and exec. pos starts from 1. lentgh is used
3762
# when type is String.
@@ -125,18 +150,30 @@ def bind_param(key, param, type = nil, length = nil)
125150
# though PL/SQL. Use OCI8::Cursor#[] explicitly to get bind
126151
# variables.
127152
def exec(*bindvars)
128-
bind_params(*bindvars)
129-
case type
130-
when :select_stmt
131-
__execute(0)
132-
define_columns() if @column_metadata.size == 0
153+
if @reparse_between_execs && @already_executed && @sql
154+
__reprepare(@sql)
155+
@bind_handles.each { |key, handle| __bind(key, handle) }
156+
@column_metadata = []
157+
@define_handles = []
158+
@names = nil
133159
@rowbuf_size = 0
134160
@rowbuf_index = 0
135-
@column_metadata.size
136-
else
137-
__execute(1)
138-
row_count
139161
end
162+
bind_params(*bindvars)
163+
result =
164+
case type
165+
when :select_stmt
166+
__execute(0)
167+
define_columns() if @column_metadata.size == 0
168+
@rowbuf_size = 0
169+
@rowbuf_index = 0
170+
@column_metadata.size
171+
else
172+
__execute(1)
173+
row_count
174+
end
175+
@already_executed = true
176+
result
140177
end
141178

142179
# Gets fetched data as array. This is available for select

test/test_all.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
require "#{srcdir}/test_connstr"
2121
require "#{srcdir}/test_metadata"
2222
require "#{srcdir}/test_array_dml"
23+
require "#{srcdir}/test_reparse_between_execs"
2324
require "#{srcdir}/test_rowid"
2425
require "#{srcdir}/test_appinfo"
2526
require "#{srcdir}/test_oracle_version"

test/test_reparse_between_execs.rb

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
require 'oci8'
2+
require File.dirname(__FILE__) + '/config'
3+
4+
class TestReparseBetweenExecs < Minitest::Test
5+
TABLE_NAME = 'rb_reparse_test'
6+
TRIGGER_NAME = 'rb_reparse_test_pkt'
7+
SEQUENCE_NAME = 'rb_reparse_test_seq'
8+
9+
def setup
10+
@conn = get_oci8_connection
11+
drop_table(TABLE_NAME)
12+
drop_sequence(SEQUENCE_NAME)
13+
@conn.exec("CREATE TABLE #{TABLE_NAME} (id NUMBER PRIMARY KEY, name VARCHAR2(50))")
14+
@conn.exec("CREATE SEQUENCE #{SEQUENCE_NAME}")
15+
@conn.exec(<<~SQL)
16+
CREATE OR REPLACE TRIGGER #{TRIGGER_NAME}
17+
BEFORE INSERT ON #{TABLE_NAME} FOR EACH ROW
18+
BEGIN
19+
IF :new.id IS NULL THEN
20+
SELECT #{SEQUENCE_NAME}.NEXTVAL INTO :new.id FROM dual;
21+
END IF;
22+
END;
23+
SQL
24+
end
25+
26+
def teardown
27+
drop_table(TABLE_NAME) if @conn
28+
drop_sequence(SEQUENCE_NAME) if @conn
29+
@conn.logoff if @conn
30+
end
31+
32+
def drop_sequence(name)
33+
@conn.exec("DROP SEQUENCE #{name}")
34+
rescue OCIError
35+
raise if $!.code != 2289 # ORA-02289: sequence does not exist
36+
end
37+
38+
# Default state: the opt-in flag is off so existing behavior is unchanged.
39+
def test_default_flag_is_false
40+
cur = @conn.parse("SELECT 1 FROM dual")
41+
assert_equal false, cur.reparse_between_execs
42+
ensure
43+
cur.close if cur
44+
end
45+
46+
# Re-executing the same parsed cursor under CURSOR_SHARING=FORCE with
47+
# both a literal and a RETURNING ... INTO :bind clause triggers an
48+
# Oracle Client TTC fast-path defect that deadlocks the session.
49+
# Setting reparse_between_execs = true must work around it: every exec
50+
# succeeds and yields a fresh sequence value via the RETURNING bind.
51+
def test_returning_into_under_cursor_sharing_force_with_flag
52+
@conn.exec("ALTER SESSION SET CURSOR_SHARING = FORCE")
53+
cur = @conn.parse(
54+
"INSERT INTO #{TABLE_NAME} (name) VALUES ('a') RETURNING id INTO :returning_id"
55+
)
56+
cur.reparse_between_execs = true
57+
cur.bind_param(1, nil, Integer)
58+
59+
ids = Array.new(3) do
60+
cur.exec
61+
cur[1]
62+
end
63+
64+
assert_equal 3, ids.size
65+
ids.each { |id| assert_kind_of Integer, id }
66+
assert_equal ids, ids.uniq,
67+
'each RETURNING value must be a fresh sequence number'
68+
ensure
69+
cur.close if cur
70+
@conn.exec("ALTER SESSION SET CURSOR_SHARING = EXACT") rescue nil
71+
end
72+
73+
# The flag must not break a plain INSERT (no RETURNING, no cursor
74+
# sharing) that re-executes the same cursor with a different bind
75+
# value each time.
76+
def test_flag_does_not_break_plain_repeated_insert
77+
cur = @conn.parse("INSERT INTO #{TABLE_NAME} (name) VALUES (:name)")
78+
cur.reparse_between_execs = true
79+
cur.bind_param(:name, nil, String, 50)
80+
81+
%w[alpha beta gamma].each do |name|
82+
cur[:name] = name
83+
cur.exec
84+
end
85+
86+
rows = []
87+
@conn.exec("SELECT name FROM #{TABLE_NAME} ORDER BY id") { |r| rows << r[0] }
88+
assert_equal %w[alpha beta gamma], rows
89+
ensure
90+
cur.close if cur
91+
end
92+
end

0 commit comments

Comments
 (0)