Merge pull request finos#2608 from finos/fix-gil-unlocking

Fix intermittent `to_arrow()` error in Python
onesimus-wiafe · Apr 30, 2024 · c39fcc4 · c39fcc4
2 parents eedb934 + f7a07a6
commit c39fcc4
Showing 1 changed file with 61 additions and 40 deletions.
diff --git a/python/perspective/perspective/src/view.cpp b/python/perspective/perspective/src/view.cpp
@@ -412,11 +412,14 @@ namespace binding {
         std::int32_t end_col,
         bool compress
     ) {
-        PSP_GIL_UNLOCK();
-        PSP_READ_LOCK(view->get_lock());
-        std::shared_ptr<std::string> str = view->to_arrow(
-            start_row, end_row, start_col, end_col, true, compress
-        );
+        std::shared_ptr<std::string> str;
+        {
+            PSP_GIL_UNLOCK();
+            PSP_READ_LOCK(view->get_lock());
+            str = view->to_arrow(
+                start_row, end_row, start_col, end_col, true, compress
+            );
+        }
         return py::bytes(*str);
     }
 
@@ -429,11 +432,14 @@ namespace binding {
         std::int32_t end_col,
         bool compress
     ) {
-        PSP_GIL_UNLOCK();
-        PSP_READ_LOCK(view->get_lock());
-        std::shared_ptr<std::string> str = view->to_arrow(
-            start_row, end_row, start_col, end_col, true, compress
-        );
+        std::shared_ptr<std::string> str;
+        {
+            PSP_GIL_UNLOCK();
+            PSP_READ_LOCK(view->get_lock());
+            str = view->to_arrow(
+                start_row, end_row, start_col, end_col, true, compress
+            );
+        }
         return py::bytes(*str);
     }
 
@@ -446,11 +452,14 @@ namespace binding {
         std::int32_t end_col,
         bool compress
     ) {
-        PSP_GIL_UNLOCK();
-        PSP_READ_LOCK(view->get_lock());
-        std::shared_ptr<std::string> str = view->to_arrow(
-            start_row, end_row, start_col, end_col, true, compress
-        );
+        std::shared_ptr<std::string> str;
+        {
+            PSP_GIL_UNLOCK();
+            PSP_READ_LOCK(view->get_lock());
+            str = view->to_arrow(
+                start_row, end_row, start_col, end_col, true, compress
+            );
+        }
         return py::bytes(*str);
     }
 
@@ -463,11 +472,14 @@ namespace binding {
         std::int32_t end_col,
         bool compress
     ) {
-        PSP_GIL_UNLOCK();
-        PSP_READ_LOCK(view->get_lock());
-        std::shared_ptr<std::string> str = view->to_arrow(
-            start_row, end_row, start_col, end_col, true, compress
-        );
+        std::shared_ptr<std::string> str;
+        {
+            PSP_GIL_UNLOCK();
+            PSP_READ_LOCK(view->get_lock());
+            str = view->to_arrow(
+                start_row, end_row, start_col, end_col, true, compress
+            );
+        }
         return py::bytes(*str);
     }
 
@@ -530,41 +542,50 @@ namespace binding {
 
     py::bytes
     get_row_delta_unit(std::shared_ptr<View<t_ctxunit>> view) {
-        PSP_GIL_UNLOCK();
-        PSP_READ_LOCK(view->get_lock());
-        std::shared_ptr<t_data_slice<t_ctxunit>> slice = view->get_row_delta();
-        std::shared_ptr<std::string> arrow =
-            view->data_slice_to_arrow(slice, false, false);
+        std::shared_ptr<std::string> arrow;
+        {
+            PSP_GIL_UNLOCK();
+            PSP_READ_LOCK(view->get_lock());
+            std::shared_ptr<t_data_slice<t_ctxunit>> slice =
+                view->get_row_delta();
+            arrow = view->data_slice_to_arrow(slice, false, false);
+        }
         return py::bytes(*arrow);
     }
 
     py::bytes
     get_row_delta_zero(std::shared_ptr<View<t_ctx0>> view) {
-        PSP_GIL_UNLOCK();
-        PSP_READ_LOCK(view->get_lock());
-        std::shared_ptr<t_data_slice<t_ctx0>> slice = view->get_row_delta();
-        std::shared_ptr<std::string> arrow =
-            view->data_slice_to_arrow(slice, false, false);
+        std::shared_ptr<std::string> arrow;
+        {
+            PSP_GIL_UNLOCK();
+            PSP_READ_LOCK(view->get_lock());
+            std::shared_ptr<t_data_slice<t_ctx0>> slice = view->get_row_delta();
+            arrow = view->data_slice_to_arrow(slice, false, false);
+        }
         return py::bytes(*arrow);
     }
 
     py::bytes
     get_row_delta_one(std::shared_ptr<View<t_ctx1>> view) {
-        PSP_GIL_UNLOCK();
-        PSP_READ_LOCK(view->get_lock());
-        std::shared_ptr<t_data_slice<t_ctx1>> slice = view->get_row_delta();
-        std::shared_ptr<std::string> arrow =
-            view->data_slice_to_arrow(slice, false, false);
+        std::shared_ptr<std::string> arrow;
+        {
+            PSP_GIL_UNLOCK();
+            PSP_READ_LOCK(view->get_lock());
+            std::shared_ptr<t_data_slice<t_ctx1>> slice = view->get_row_delta();
+            arrow = view->data_slice_to_arrow(slice, false, false);
+        }
         return py::bytes(*arrow);
     }
 
     py::bytes
     get_row_delta_two(std::shared_ptr<View<t_ctx2>> view) {
-        PSP_GIL_UNLOCK();
-        PSP_READ_LOCK(view->get_lock());
-        std::shared_ptr<t_data_slice<t_ctx2>> slice = view->get_row_delta();
-        std::shared_ptr<std::string> arrow =
-            view->data_slice_to_arrow(slice, false, false);
+        std::shared_ptr<std::string> arrow;
+        {
+            PSP_GIL_UNLOCK();
+            PSP_READ_LOCK(view->get_lock());
+            std::shared_ptr<t_data_slice<t_ctx2>> slice = view->get_row_delta();
+            arrow = view->data_slice_to_arrow(slice, false, false);
+        }
         return py::bytes(*arrow);
     }