Skip to content

Commit

Permalink
Don't increment reference count in Py_BuildValue. (#12)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
robertnishihara authored and pcmoritz committed Sep 6, 2016
1 parent 2e4b655 commit 5ac2df4
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 9 deletions.
5 changes: 3 additions & 2 deletions python/src/pynumbuf/adapters/numpy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
32 changes: 25 additions & 7 deletions python/src/pynumbuf/adapters/python.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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();
Expand Down Expand Up @@ -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();
}

Expand All @@ -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();
Expand Down

0 comments on commit 5ac2df4

Please sign in to comment.