Skip to content

Commit 541ad43

Browse files
committed
PYCBC-1813: Fix async txn callback leaks and add unraisable
Changes -------- * Add complete_pending_operation() as the shared tail of every async txn completion path, folding in handle_returning_void, handle_returning_transaction_get_result, handle_returning_transaction_get_multi_result, handle_returning_query_result, and transaction_commit's inline completion lambda; report a raised callback/errback via PyErr_WriteUnraisable instead of dropping it silently * Fix transaction_op's 7 early-return paths and its default: case to Py_XDECREF the Py_XINCREF'd callback/errback before returning; the default: case also now returns instead of falling through, which previously hung the sync path on fut.get() waiting on a promise nothing would set * Fix the same leak in transaction_get_multi_op's analogous unrecognized-op_type fallback * Add PyErr_WriteUnraisable at the 5 remaining connection.cxx/ connection.hxx sites where a raised user callback/errback was discarded unreported Change-Id: I416d7e4319f41b338b7e335853734d8942a7d7d7 Reviewed-on: https://review.couchbase.org/c/couchbase-python-client/+/250600 Tested-by: Build Bot <build@couchbase.com> Reviewed-by: Dimitris Christodoulou <dimitris.christodoulou@couchbase.com>
1 parent 56f4fcb commit 541ad43

3 files changed

Lines changed: 112 additions & 142 deletions

File tree

src/connection.cxx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ Connection::handle_connection_operation_callback(std::error_code ec,
113113
}
114114
} else if (pyObj_errback != nullptr) {
115115
PyObject* ret = PyObject_CallFunctionObjArgs(pyObj_errback, result, nullptr);
116+
if (ret == nullptr) {
117+
// errback raised; nothing else observes this IO thread's exception state.
118+
PyErr_WriteUnraisable(pyObj_errback);
119+
}
116120
Py_XDECREF(ret);
117121
Py_XDECREF(result);
118122
} else if (barrier) {
@@ -132,6 +136,10 @@ Connection::handle_connection_operation_callback(std::error_code ec,
132136

133137
if (pyObj_callback != nullptr) {
134138
PyObject* ret = PyObject_CallFunctionObjArgs(pyObj_callback, result, nullptr);
139+
if (ret == nullptr) {
140+
// callback raised; nothing else observes this IO thread's exception state.
141+
PyErr_WriteUnraisable(pyObj_callback);
142+
}
135143
Py_XDECREF(ret);
136144
Py_XDECREF(result);
137145
} else if (barrier) {

src/connection.hxx

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,10 @@ private:
142142
}
143143
} else if (pyObj_callback != nullptr) {
144144
PyObject* ret = PyObject_CallFunctionObjArgs(pyObj_callback, result, nullptr);
145+
if (ret == nullptr) {
146+
// callback raised; nothing else observes this IO thread's exception state.
147+
PyErr_WriteUnraisable(pyObj_callback);
148+
}
145149
Py_XDECREF(ret);
146150
Py_XDECREF(result);
147151
} else if (barrier) {
@@ -265,7 +269,12 @@ private:
265269
PyObject_TypeCheck(result, &pycbc_exception_type) ? pyObj_errback : pyObj_callback;
266270

267271
if (target_handler != nullptr) {
268-
Py_XDECREF(PyObject_CallFunctionObjArgs(target_handler, result, nullptr));
272+
PyObject* ret = PyObject_CallFunctionObjArgs(target_handler, result, nullptr);
273+
if (ret == nullptr) {
274+
// callback/errback raised; nothing else observes this IO thread's exception state.
275+
PyErr_WriteUnraisable(target_handler);
276+
}
277+
Py_XDECREF(ret);
269278
} else if (barrier != nullptr) {
270279
barrier->set_value(result);
271280
result = nullptr; // Reference transferred to barrier
@@ -703,6 +712,10 @@ Connection::execute_streaming_op(PyObject* kwargs)
703712
// The python side will determine if the query has rows or returned an error
704713
if (callback != nullptr) {
705714
PyObject* result = PyObject_CallFunction(callback, "O", Py_True);
715+
if (result == nullptr) {
716+
// callback raised; nothing else observes this IO thread's exception state.
717+
PyErr_WriteUnraisable(callback);
718+
}
706719
Py_XDECREF(result);
707720
}
708721

0 commit comments

Comments
 (0)