Skip to content

Commit

Permalink
connect: Do not deref encoding parameter (#1343)
Browse files Browse the repository at this point in the history
The `encoding` parameter was being put into an Object wrapper which decremented its refcount
causing a segfault if used.  The value is not kept so the easiest solution is to simply borrow
the original pointer and do not wrap it.  I'm not sure why I did.
  • Loading branch information
mkleehammer committed Apr 16, 2024
1 parent ff1dd23 commit 9078f19
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 5 deletions.
6 changes: 3 additions & 3 deletions src/connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ static char* StrDup(const char* text) {
}


static bool Connect(PyObject* pConnectString, HDBC hdbc, long timeout, Object& encoding)
static bool Connect(PyObject* pConnectString, HDBC hdbc, long timeout, PyObject* encoding)
{
assert(PyUnicode_Check(pConnectString));

Expand Down Expand Up @@ -170,7 +170,7 @@ static bool ApplyPreconnAttrs(HDBC hdbc, SQLINTEGER ikey, PyObject *value, char
}

PyObject* Connection_New(PyObject* pConnectString, bool fAutoCommit, long timeout, bool fReadOnly,
PyObject* attrs_before, Object& encoding)
PyObject* attrs_before, PyObject* encoding)
{
//
// Allocate HDBC and connect
Expand All @@ -196,7 +196,7 @@ PyObject* Connection_New(PyObject* pConnectString, bool fAutoCommit, long timeou
PyObject* value = 0;

Object encodingholder;
char *strencoding = encoding.Get() ?
char *strencoding = (encoding != 0) ?
(PyUnicode_Check(encoding) ? PyBytes_AsString(encodingholder = PyCodec_Encode(encoding, "utf-8", "strict")) :
PyBytes_Check(encoding) ? PyBytes_AsString(encoding) : 0) : 0;

Expand Down
2 changes: 1 addition & 1 deletion src/connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ struct Connection
* exception is set and zero is returned.
*/
PyObject* Connection_New(PyObject* pConnectString, bool fAutoCommit, long timeout, bool fReadOnly,
PyObject* attrs_before, Object& encoding);
PyObject* attrs_before, PyObject* encoding);

/*
* Used by the Cursor to implement commit and rollback.
Expand Down
2 changes: 1 addition & 1 deletion src/pyodbcmodule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ static PyObject* mod_connect(PyObject* self, PyObject* args, PyObject* kwargs)
int fAutoCommit = 0;
int fReadOnly = 0;
long timeout = 0;
Object encoding;
PyObject* encoding = 0;

Object attrs_before; // Optional connect attrs set before connecting

Expand Down
27 changes: 27 additions & 0 deletions tests/postgresql_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -593,3 +593,30 @@ def convert(value):
cursor.connection.add_output_converter(pyodbc.SQL_WVARCHAR, None)
value = cursor.execute("select v from t1").fetchone()[0]
assert value == '123.45'


def test_refcount_encoding():
"""
Ensure we handle the reference count to `encoding` properly. In the past we freed a
borrowed reference. This would cause a segfault.
"""
# https://github.com/mkleehammer/pyodbc/issues/1343
import sys
encoding = 'utf-16le'
count_before = sys.getrefcount(encoding)

def _test():
# I've moved this into a function so the exception's stack trace will be freed under
# the covers when we leave the function. Otherwise we'd have a 2nd reference to
# `encoding` in the stack trace of the exception.
try:
cnxn = pyodbc.connect(CNXNSTR, encoding=encoding)
except:
pass

for i in range(10):
_test()

count_after = sys.getrefcount(encoding)

assert count_after == count_before

0 comments on commit 9078f19

Please sign in to comment.