From 5ac2df4329d2dc3039a503151ab985067e28c733 Mon Sep 17 00:00:00 2001
From: Robert Nishihara <robertnishihara@gmail.com>
Date: Tue, 6 Sep 2016 10:02:30 -0700
Subject: [PATCH] Don't increment reference count in Py_BuildValue. (#12)

* Decrement reference count for result of serialization callback and also for an object created in DeserializeDict.

* Check tuples exactly so named tuples go to callback.

* Assert that callback is set if _pytype_ key present.
---
 python/src/pynumbuf/adapters/numpy.cc  |  5 ++--
 python/src/pynumbuf/adapters/python.cc | 32 ++++++++++++++++++++------
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/python/src/pynumbuf/adapters/numpy.cc b/python/src/pynumbuf/adapters/numpy.cc
index e3e1fa366..3ae7170a3 100644
--- a/python/src/pynumbuf/adapters/numpy.cc
+++ b/python/src/pynumbuf/adapters/numpy.cc
@@ -114,14 +114,15 @@ Status SerializeArray(PyArrayObject* array, SequenceBuilder& builder,
         return Status::NotImplemented(stream.str());
       } else {
         PyObject* arglist = Py_BuildValue("(O)", array);
+        // The reference count of the result of the call to PyObject_CallObject
+        // must be decremented. This is done in SerializeDict in python.cc.
         PyObject* result = PyObject_CallObject(numbuf_serialize_callback, arglist);
+        Py_XDECREF(arglist);
         if (!result) {
-          Py_XDECREF(arglist);
           return Status::NotImplemented("python error"); // TODO(pcm): https://github.com/pcmoritz/numbuf/issues/10
         }
         builder.AppendDict(PyDict_Size(result));
         subdicts.push_back(result);
-        Py_XDECREF(arglist);
       }
   }
   Py_XDECREF(contiguous);
diff --git a/python/src/pynumbuf/adapters/python.cc b/python/src/pynumbuf/adapters/python.cc
index 5b71865ff..7044b5c93 100644
--- a/python/src/pynumbuf/adapters/python.cc
+++ b/python/src/pynumbuf/adapters/python.cc
@@ -96,7 +96,7 @@ Status append(PyObject* elem, SequenceBuilder& builder,
   } else if (PyDict_Check(elem)) {
     builder.AppendDict(PyDict_Size(elem));
     subdicts.push_back(elem);
-  } else if (PyTuple_Check(elem)) {
+  } else if (PyTuple_CheckExact(elem)) {
     builder.AppendTuple(PyTuple_Size(elem));
     subtuples.push_back(elem);
   } else if (PyArray_IsScalar(elem, Generic)) {
@@ -113,14 +113,15 @@ Status append(PyObject* elem, SequenceBuilder& builder,
       return Status::NotImplemented(ss.str());
     } else {
       PyObject* arglist = Py_BuildValue("(O)", elem);
+      // The reference count of the result of the call to PyObject_CallObject
+      // must be decremented. This is done in SerializeDict in this file.
       PyObject* result = PyObject_CallObject(numbuf_serialize_callback, arglist);
+      Py_XDECREF(arglist);
       if (!result) {
-        Py_XDECREF(arglist);
         return Status::NotImplemented("python error"); // TODO(pcm): https://github.com/pcmoritz/numbuf/issues/10
       }
       builder.AppendDict(PyDict_Size(result));
       subdicts.push_back(result);
-      Py_XDECREF(arglist);
     }
   }
   return Status::OK();
@@ -219,6 +220,20 @@ Status SerializeDict(std::vector<PyObject*> dicts, std::shared_ptr<Array>* out)
     RETURN_NOT_OK(SerializeDict(val_dicts, &val_dict_arr));
   }
   *out = result.Finish(key_tuples_arr, val_list_arr, val_tuples_arr, val_dict_arr);
+
+  // This block is used to decrement the reference counts of the results
+  // returned by the serialization callback, which is called in SerializeArray
+  // in numpy.cc as well as in DeserializeDict and in append in this file.
+  static PyObject* py_type = PyString_FromString("_pytype_");
+  for (const auto& dict : dicts) {
+    if (PyDict_Contains(dict, py_type)) {
+      // If the dictionary contains the key "_pytype_", then the user has to
+      // have registered a callback.
+      ARROW_CHECK(numbuf_serialize_callback);
+      Py_XDECREF(dict);
+    }
+  }
+
   return Status::OK();
 }
 
@@ -237,12 +252,15 @@ Status DeserializeDict(std::shared_ptr<Array> array, int32_t start_idx, int32_t
   static PyObject* py_type = PyString_FromString("_pytype_");
   if (PyDict_Contains(result, py_type) && numbuf_deserialize_callback) {
     PyObject* arglist = Py_BuildValue("(O)", result);
-    result = PyObject_CallObject(numbuf_deserialize_callback, arglist);
-    if (!result) {
-      Py_XDECREF(arglist);
+    // The result of the call to PyObject_CallObject will be passed to Python
+    // and its reference count will be decremented by the interpreter.
+    PyObject* callback_result = PyObject_CallObject(numbuf_deserialize_callback, arglist);
+    Py_XDECREF(arglist);
+    Py_XDECREF(result);
+    result = callback_result;
+    if (!callback_result) {
       return Status::NotImplemented("python error"); // TODO(pcm): https://github.com/pcmoritz/numbuf/issues/10
     }
-    Py_XDECREF(arglist);
   }
   *out = result;
   return Status::OK();