PYCBC-1820: Add C++ exception boundaries at extension entry points

Changes
--------
* Wrap execute_multi_op's body in try/catch(const std::exception&),
  matching execute_kv_op's existing shape, so a throw from
  py_to_cbpp (e.g. invalid-UTF-8 bucket/scope/collection/key) can't
  unwind uncaught into CPython
* Convert every Py_BEGIN/END_ALLOW_THREADS pair in transactions.cxx
  (21 across the 9 module-level entry points plus
  dealloc_transactions) to pycbc::gil_release_guard, and wrap each in
  try/catch: the guard restores the GIL during unwind before the
  catch runs, which the bare macro pair doesn't
* Dispatch-call failures raise_invalid_argument after releasing any
  already-incref'd callback/errback refs; sync fut.get() failures
  use set_runtime_error_if_unset; dealloc_transactions logs and
  swallows, since a dealloc can't propagate an exception
* Fix create_transaction_context leaking py_ctx on a PyCapsule_New
  failure, found while adding its boundary

Change-Id: Ia575112e0b59ec2139151a86ae247eadf913cbee
Reviewed-on: https://review.couchbase.org/c/couchbase-python-client/+/250614
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dimitris Christodoulou <dimitris.christodoulou@couchbase.com>

Author: thejcfactor

src/connection.hxx

@@ -496,83 +496,88 @@ Connection::execute_multi_op(PyObject* arg)
   }
   pycbc_result* multi_result = reinterpret_cast<pycbc_result*>(pyObj_multi_result);
 
-  for (size_t i = 0; i < num_docs; ++i) {
-    PyObject* pyObj_binding = PyList_GetItem(arg, i); // Borrowed ref
-    // Unchecked by contract, see validate_connection_and_multi_request
-    pycbc_kv_request* request = reinterpret_cast<pycbc_kv_request*>(pyObj_binding);
-    std::string key_str = py_to_cbpp<std::string>(request->key);
-    std::shared_ptr<couchbase::core::tracing::wrapper_sdk_span> wrapper_span;
-    std::string span_name;
-    extract_field(request->wrapper_span_name, span_name);
-    if (!span_name.empty()) {
-      wrapper_span = std::make_shared<couchbase::core::tracing::wrapper_sdk_span>(span_name);
-    }
-
-    auto req = py_to_cbpp<Request>(request, wrapper_span);
-    if (PyErr_Occurred()) {
-      Py_DECREF(pyObj_multi_result);
-      return nullptr;
-    }
+  try {
+    for (size_t i = 0; i < num_docs; ++i) {
+      PyObject* pyObj_binding = PyList_GetItem(arg, i); // Borrowed ref
+      // Unchecked by contract, see validate_connection_and_multi_request
+      pycbc_kv_request* request = reinterpret_cast<pycbc_kv_request*>(pyObj_binding);
+      std::string key_str = py_to_cbpp<std::string>(request->key);
+      std::shared_ptr<couchbase::core::tracing::wrapper_sdk_span> wrapper_span;
+      std::string span_name;
+      extract_field(request->wrapper_span_name, span_name);
+      if (!span_name.empty()) {
+        wrapper_span = std::make_shared<couchbase::core::tracing::wrapper_sdk_span>(span_name);
+      }
 
-    // TODO(PYCBC-1746): Delete w/ removal of legacy tracing logic
-    if (wrapper_span == nullptr) {
-      add_cluster_labels(req);
-    }
+      auto req = py_to_cbpp<Request>(request, wrapper_span);
+      if (PyErr_Occurred()) {
+        Py_DECREF(pyObj_multi_result);
+        return nullptr;
+      }
 
-    std::optional<std::chrono::system_clock::time_point> start_time;
-    if (request->with_metrics == Py_True) {
-      start_time = std::chrono::system_clock::now();
-    }
+      // TODO(PYCBC-1746): Delete w/ removal of legacy tracing logic
+      if (wrapper_span == nullptr) {
+        add_cluster_labels(req);
+      }
 
-    auto barrier = std::make_shared<std::promise<Response>>();
-    auto fut = barrier->get_future();
+      std::optional<std::chrono::system_clock::time_point> start_time;
+      if (request->with_metrics == Py_True) {
+        start_time = std::chrono::system_clock::now();
+      }
 
-    staging.push_back({ std::move(req),
-                        std::move(key_str),
-                        std::move(wrapper_span),
-                        start_time,
-                        std::move(barrier),
-                        std::move(fut) });
-  }
+      auto barrier = std::make_shared<std::promise<Response>>();
+      auto fut = barrier->get_future();
 
-  {
-    gil_release_guard no_gil;
-    for (auto& s : staging) {
-      auto barrier = s.barrier;
-      cluster_.execute(s.req, [barrier](Response resp) {
-        barrier->set_value(std::move(resp));
-      });
+      staging.push_back({ std::move(req),
+                          std::move(key_str),
+                          std::move(wrapper_span),
+                          start_time,
+                          std::move(barrier),
+                          std::move(fut) });
     }
 
-    for (auto& s : staging) {
-      s.fut.wait();
-    }
-  }
+    {
+      gil_release_guard no_gil;
+      for (auto& s : staging) {
+        auto barrier = s.barrier;
+        cluster_.execute(s.req, [barrier](Response resp) {
+          barrier->set_value(std::move(resp));
+        });
+      }
 
-  bool all_okay = true;
-  for (auto& s : staging) {
-    PyObject* res =
-      finalize_kv_result<Request>(s.fut.get(), std::move(s.wrapper_span), std::move(s.start_time));
-    // OOM is not a per-key condition, so abandon the whole multi result rather than
-    // reporting a partial one. An exception is already pending.
-    if (res == nullptr) {
-      Py_DECREF(pyObj_multi_result);
-      return nullptr;
+      for (auto& s : staging) {
+        s.fut.wait();
+      }
     }
-    if (PyObject_TypeCheck(res, &pycbc_exception_type)) {
-      all_okay = false;
+
+    bool all_okay = true;
+    for (auto& s : staging) {
+      PyObject* res = finalize_kv_result<Request>(
+        s.fut.get(), std::move(s.wrapper_span), std::move(s.start_time));
+      // OOM is not a per-key condition, so abandon the whole multi result rather than
+      // reporting a partial one. An exception is already pending.
+      if (res == nullptr) {
+        Py_DECREF(pyObj_multi_result);
+        return nullptr;
+      }
+      if (PyObject_TypeCheck(res, &pycbc_exception_type)) {
+        all_okay = false;
+      }
+      int rc = PyDict_SetItemString(multi_result->raw_result, s.key_str.c_str(), res);
+      Py_DECREF(res);
+      if (rc < 0) {
+        Py_DECREF(pyObj_multi_result);
+        return nullptr;
+      }
     }
-    int rc = PyDict_SetItemString(multi_result->raw_result, s.key_str.c_str(), res);
-    Py_DECREF(res);
-    if (rc < 0) {
+    if (PyDict_SetItemString(multi_result->raw_result, "all_okay", all_okay ? Py_True : Py_False) <
+        0) {
       Py_DECREF(pyObj_multi_result);
       return nullptr;
     }
-  }
-  if (PyDict_SetItemString(multi_result->raw_result, "all_okay", all_okay ? Py_True : Py_False) <
-      0) {
+  } catch (const std::exception& e) {
     Py_DECREF(pyObj_multi_result);
-    return nullptr;
+    return raise_invalid_argument(e.what());
   }
 
   return pyObj_multi_result;