oracle / python-oracledb

Python driver for Oracle Database conforming to the Python DB API 2.0 specification. This is the renamed, new major release of cx_Oracle
https://oracle.github.io/python-oracledb
Other
342 stars 69 forks source link

Thin mode - Segmentation Fault when using oracledb.CURSOR #368

Closed mkmoisen closed 3 months ago

mkmoisen commented 4 months ago
  1. What versions are you using?

Oracle 19

Also run Python and show the output of:

>>> import sys
>>> import platform
>>> 
>>> print("platform.platform:", platform.platform())
platform.platform: Linux-4.18.0-372.76.1.el8_6.x86_64-x86_64-with-glibc2.28
>>> print("sys.maxsize > 2**32:", sys.maxsize > 2**32)
sys.maxsize > 2**32: True
>>> print("platform.python_version:", platform.python_version())
platform.python_version: 3.12.4

And:

>>> import oracledb
>>> print("oracledb.__version__:", oracledb.__version__)
oracledb.__version__: 2.2.1

-->

  1. Is it an error or a hang or a crash?

Crash

  1. What error(s) or behavior you are seeing?

Segmentation Fault

Cut and paste text showing the command you ran. No screenshots.

Use a gist for long screen output and logs: see https://gist.github.com/

-->

  1. Does your application call init_oracle_client()?

This bug only occurs in thin mode.

I cannot replicate it in thick mode.

  1. Include a runnable Python script that shows the problem.
import oracledb

conn = oracledb.connect(...)

cur = conn.cursor()

cursor_var = cur.var(oracledb.CURSOR)

cur.execute(
    '''
        DECLARE
            l_cursor SYS_REFCURSOR;
        BEGIN
            OPEN l_cursor FOR 
                SELECT 1 foo
                FROM dual
            ;

            :cursor := l_cursor;
        END;
    ''',
    dict(
        cursor=cursor_var
    )
)

cursor = cursor_var.getvalue()

rows = cursor.fetchall()

assert rows == [(1,)]

cursor.close()

# Recalling cur.execute with the same cursor_var will result in the segmentation fault.

cur.execute(
    '''
        DECLARE
            l_cursor SYS_REFCURSOR;
        BEGIN
            OPEN l_cursor FOR 
                SELECT 1 foo
                FROM dual
            ;

            :cursor := l_cursor;
        END;
    ''',
    dict(
        cursor=cursor_var
    )
)
Segmentation fault (core dumped)

No segfault offcurs when I instead instantiate a new cursor_var:

cursor.close()
cursor_var = cur.var(oracledb.CURSOR)
cur.execute(..., dict(cursor=cursor_var))

In thick mode, it works as expected.

anthony-tuininga commented 4 months ago

Thanks. I can replicate the issue. The source of the issue is your call to cursor.close() which makes the cursor unusable. Removing that call resolves the issue. I will ensure that a better error message is raised!

mkmoisen commented 4 months ago

Hi @anthony-tuininga

Thank you.

I just tried again in thick mode, please ignore my previous statement where I said it works without any issue. In thick mode it raises this error:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/foo/python3.12/site-packages/oracledb/cursor.py", line 701, in execute
    impl.execute(self)
  File "src/oracledb/impl/thick/cursor.pyx", line 306, in oracledb.thick_impl.ThickCursorImpl.execute
  File "src/oracledb/impl/thick/utils.pyx", line 456, in oracledb.thick_impl._raise_from_odpi
  File "src/oracledb/impl/thick/utils.pyx", line 446, in oracledb.thick_impl._raise_from_info
oracledb.exceptions.DatabaseError: DPI-1002: invalid OCI handle

I suppose this would be a backwards breaking change, but a nicer error message here in thick mode would be nice, as well as a consistent error message between thick and thin mode.

Thanks!

anthony-tuininga commented 4 months ago

I see that with thick mode it works if the exact statement is re-executed but fails with the invalid OCI handle error when a different statement is executed. Why do you say that it is a backwards breaking change? What version had different behavior? And what was that behavior?

mkmoisen commented 4 months ago

@anthony-tuininga

Sorry for the confusion, I mean if you change the error message to something more friendly such as "Please use a new cursor after closing it" instead of "invalid OCI handle" it would technically be a backwards breaking change.

Although I doubt anyone is writing a try/except that depends on this message.

anthony-tuininga commented 4 months ago

Ah! That makes more sense. :-) I may be able to avoid the exception completely but I need to look closely at the code to see if that is possible.

anthony-tuininga commented 4 months ago

This patch resolves the segfault:

diff --git a/src/oracledb/impl/thin/messages.pyx b/src/oracledb/impl/thin/messages.pyx
index 6679adc2..172b6008 100644
--- a/src/oracledb/impl/thin/messages.pyx
+++ b/src/oracledb/impl/thin/messages.pyx
@@ -1082,6 +1082,8 @@ cdef class MessageWithData(Message):
             buf.write_binary_float(value)
         elif ora_type_num == TNS_DATA_TYPE_CURSOR:
             cursor_impl = value._impl
+            if cursor_impl is None:
+                errors._raise_err(errors.ERR_CURSOR_NOT_OPEN)
             if cursor_impl._statement is None:
                 cursor_impl._statement = self.conn_impl._get_statement()
             if cursor_impl._statement._cursor_id == 0:

To check this in thick mode requires considerably more effort. Avoiding the exception is not possible because in thick mode it is impossible to tell whether a PL/SQL bind variable is IN, IN/OUT or OUT -- so an exception should be thrown if the cursor that is bound has been closed.

anthony-tuininga commented 4 months ago

I have pushed a patch that corrects this issue. If you are able to build from source you can verify that it works for you, too.

anthony-tuininga commented 3 months ago

This was included in python-oracledb 2.4.0 which was just released.