8000 BUG: test, fix PyArray_DiscardWritebackIfCopy refcount issue and docu… by mattip · Pull Request #10824 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

BUG: test, fix PyArray_DiscardWritebackIfCopy refcount issue and docu… #10824

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 15 additions & 11 deletions doc/source/reference/c-api.array.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1360,7 +1360,7 @@ Special functions for NPY_OBJECT
.. c:function:: int PyArray_SetWritebackIfCopyBase(PyArrayObject* arr, PyArrayObject* base)

Precondition: ``arr`` is a copy of ``base`` (though possibly with different
strides, ordering, etc.) Sets the :c:data:`NPY_ARRAY_WRITEBACKIFCOPY` flag
strides, ordering, etc.) Sets the :c:data:`NPY_ARRAY_WRITEBACKIFCOPY` flag
and ``arr->base``, and set ``base`` to READONLY. Call
:c:func:`PyArray_ResolveWritebackIfCopy` before calling
`Py_DECREF`` in order copy any changes back to ``base`` and
Expand Down Expand Up @@ -3260,12 +3260,14 @@ Memory management
.. c:function:: int PyArray_ResolveWritebackIfCopy(PyArrayObject* obj)

If ``obj.flags`` has :c:data:`NPY_ARRAY_WRITEBACKIFCOPY` or (deprecated)
:c:data:`NPY_ARRAY_UPDATEIFCOPY`, this function copies ``obj->data`` to
`obj->base->data`, clears the flags, `DECREF` s `obj->base` and makes it
writeable, and sets ``obj->base`` to NULL. This is the opposite of
:c:data:`NPY_ARRAY_UPDATEIFCOPY`, this function clears the flags, `DECREF` s
`obj->base` and makes it writeable, and sets ``obj->base`` to NULL. It then
copies ``obj->data`` to `obj->base->data`, and returns the error state of
the copy operation. This is the opposite of
:c:func:`PyArray_SetWritebackIfCopyBase`. Usually this is called once
you are finished with ``obj``, just before ``Py_DECREF(obj)``. It may be called
multiple times, or with ``NULL`` input.
multiple times, or with ``NULL`` input. See also
:c:func:`PyArray_DiscardWritebackIfCopy`.

Returns 0 if nothing was done, -1 on error, and 1 if action was taken.

Expand Down Expand Up @@ -3487,12 +3489,14 @@ Miscellaneous Macros

.. c:function:: PyArray_DiscardWritebackIfCopy(PyObject* obj)

Reset the :c:data:`NPY_ARRAY_WRITEBACKIFCOPY` and deprecated
:c:data:`NPY_ARRAY_UPDATEIFCOPY` flag. Resets the
:c:data:`NPY_ARRAY_WRITEABLE` flag on the base object. It also
discards pending changes to the base object. This is
useful for recovering from an error condition when
writeback semantics are used.
If ``obj.flags`` has :c:data:`NPY_ARRAY_WRITEBACKIFCOPY` or (deprecated)
:c:data:`NPY_ARRAY_UPDATEIFCOPY`, this function clears the flags, `DECREF` s
`obj->base` and makes it writeable, and sets ``obj->base`` to NULL. In
contrast to :c:func:`PyArray_DiscardWritebackIfCopy` it makes no attempt
to copy the data from `obj->base` This undoes
:c:func:`PyArray_SetWritebackIfCopyBase`. Usually this is called after an
error when you are finished with ``obj``, just before ``Py_DECREF(obj)``.
It may be called multiple times, or with ``NULL`` input.

.. c:function:: PyArray_XDECREF_ERR(PyObject* obj)

Expand Down
13 changes: 8 additions & 5 deletions numpy/core/include/numpy/ndarrayobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,14 +170,17 @@ extern "C" CONFUSE_EMACS
(k)*PyArray_STRIDES(obj)[2] + \
(l)*PyArray_STRIDES(obj)[3]))

/* Move to arrayobject.c once PyArray_XDECREF_ERR is removed */
static NPY_INLINE void
PyArray_DiscardWritebackIfCopy(PyArrayObject *arr)
{
if (arr != NULL) {
if ((PyArray_FLAGS(arr) & NPY_ARRAY_WRITEBACKIFCOPY) ||
(PyArray_FLAGS(arr) & NPY_ARRAY_UPDATEIFCOPY)) {
PyArrayObject *base = (PyArrayObject *)PyArray_BASE(arr);
PyArray_ENABLEFLAGS(base, NPY_ARRAY_WRITEABLE);
PyArrayObject_fields *fa = (PyArrayObject_fields *)arr;
if (fa && fa->base) {
if ((fa->flags & NPY_ARRAY_UPDATEIFCOPY) ||
(fa->flags & NPY_ARRAY_WRITEBACKIFCOPY)) {
PyArray_ENABLEFLAGS((PyArrayObject*)fa->base, NPY_ARRAY_WRITEABLE);
Py_DECREF(fa->base);
fa->base = NULL;
PyArray_CLEARFLAGS(arr, NPY_ARRAY_WRITEBACKIFCOPY);
PyArray_CLEARFLAGS(arr, NPY_ARRAY_UPDATEIFCOPY);
}
Expand Down
15 changes: 15 additions & 0 deletions numpy/core/src/multiarray/_multiarray_tests.c.src
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,18 @@ npy_resolve(PyObject* NPY_UNUSED(self), PyObject* args)
Py_RETURN_NONE;
}

/* resolve WRITEBACKIFCOPY */
static PyObject*
npy_discard(PyObject* NPY_UNUSED(self), PyObject* args)
{
if (!PyArray_Check(args)) {
PyErr_SetString(PyExc_TypeError, "test needs ndarray input");
return NULL;
}
PyArray_DiscardWritebackIfCopy((PyArrayObject*)args);
Py_RETURN_NONE;
}

#if !defined(NPY_PY3K)
static PyObject *
int_subclass(PyObject *dummy, PyObject *args)
Expand Down Expand Up @@ -1857,6 +1869,9 @@ static PyMethodDef Multiarray_TestsMethods[] = {
{"npy_resolve",
npy_resolve,
METH_O, NULL},
{"npy_discard",
npy_discard,
METH_O, NULL},
#if !defined(NPY_PY3K)
{"test_int_subclass",
int_subclass,
Expand Down
34 changes: 31 additions & 3 deletions numpy/core/tests/test_multiarray.py
Original file line number Diff line number Diff line change
Expand Up @@ -7246,16 +7246,20 @@ def test_dot_out(self):

def test_view_assign(self):
from numpy.core._multiarray_tests import npy_create_writebackifcopy, npy_resolve

arr = np.arange(9).reshape(3, 3).T
arr_wb = npy_create_writebackifcopy(arr)
assert_(arr_wb.flags.writebackifcopy)
assert_(arr_wb.base is arr)
arr_wb[:] = -100
arr_wb[...] = -100
npy_resolve(arr_wb)
# arr changes after resolve, even though we assigned to arr_wb
assert_equal(arr, -100)
# after resolve, the two arrays no longer reference each other
assert_(not arr_wb.ctypes.data == 0)
arr_wb[:] = 100
assert_(arr_wb.ctypes.data != 0)
assert_equal(arr_wb.base, None)
# assigning to arr_wb does not get transfered to arr
arr_wb[...] = 100
assert_equal(arr, -100)

def test_dealloc_warning(self):
Expand All @@ -7266,6 +7270,30 @@ def test_dealloc_warning(self):
_multiarray_tests.npy_abuse_writebackifcopy(v)
assert len(sup.log) == 1

def test_view_discard_refcount(self):
from numpy.core._multiarray_tests import npy_create_writebackifcopy, npy_discard

arr = np.arange(9).reshape(3, 3).T
orig = arr.copy()
if HAS_REFCOUNT:
arr_cnt = sys.getrefcount(arr)
arr_wb = npy_create_writebackifcopy(arr)
assert_(arr_wb.flags.writebackifcopy)
assert_(arr_wb.base is arr)
arr_wb[...] = -100
npy_discard(arr_wb)
# arr remains unchanged after discard
assert_equal(arr, orig)
# after discard, the two arrays no longer reference each other
assert_(arr_wb.ctypes.data != 0)
assert_equal(arr_wb.base, None)
if HAS_REFCOUNT:
assert_equal(arr_cnt, sys.getrefcount(arr))
# assigning to arr_wb does not get transfered to arr
arr_wb[...] = 100
assert_equal(arr, orig)


class TestArange(object):
def test_infinite(self):
assert_raises_regex(
Expand Down
0