Crash on fastputmask on win32 (Trac #1032) #1630

thouis · 2012-10-19T20:44:06Z

Original ticket http://projects.scipy.org/numpy/ticket/1032 on 2009-03-02 by @cournape, assigned to @cournape.

The test suite crashes in PyArray_FastPutMask, since revision r6528. I don't see how an additional check could causes a segfault, so I guess it just show a previously undetected bug.

thouis · 2012-10-19T20:44:06Z

@cournape wrote on 2009-03-02

The offending test code:

#!python
import numpy as np
from numpy.ma.testutils import *
import numpy.ma as ma

from numpy.ma.mrecords import mrecarray

ilist = [1,2,3,4,5]
flist = [1.1,2.2,3.3,4.4,5.5]
slist = ['one','two','three','four','five']
ddtype = [('a',int),('b',float),('c','|S8')]
mask = [0,1,0,0,1]
base = ma.array(zip(ilist,flist,slist), mask=mask, dtype=ddtype)

mbase = base.view(mrecarray)
mbase_first = mbase[0]
while 1:
    assert_equal(mbase_first.tolist(), (1,1.1,'one'))

thouis · 2012-10-19T20:44:07Z

@cournape wrote on 2009-03-02

Interestingly, by forcing the offending codepath, I get a memory leak on linux for the exact same test code:

#!c
static PyObject *
PyArray_PutMask(PyArrayObject *self, PyObject* values0, PyObject* mask0)
{
    PyArray_FastPutmaskFunc *func;
    PyArrayObject  *mask, *values;
    int i, chunk, ni, max_item, nv, tmp;
    char *src, *dest;
    int copied = 0;

    mask = NULL;
    values = NULL;
    if (!PyArray_Check(self)) {
        PyErr_SetString(PyExc_TypeError,
                        "putmask: first argument must "\
                        "be an array");
        return NULL;
    }
    //if (!PyArray_ISCONTIGUOUS(self) || !(self->flags & ALIGNED)) 
    {
        PyArrayObject *obj;
        int flags = NPY_CARRAY | NPY_UPDATEIFCOPY;

        Py_INCREF(self->descr);
        obj = (PyArrayObject *)PyArray_FromArray(self,
                                                 self->descr, flags);
        if (obj != self) {
            copied = 1;
        }
        self = obj;
    }

    max_item = PyArray_SIZE(self);
    dest = self->data;
    chunk = self->descr->elsize;
    mask = (PyArrayObject *)\
        PyArray_FROM_OTF(mask0, PyArray_BOOL, CARRAY | FORCECAST);
    if (mask == NULL) {
        goto fail;
    }
    ni = PyArray_SIZE(mask);
    if (ni != max_item) {
        PyErr_SetString(PyExc_ValueError,
                        "putmask: mask and data must be "\
                        "the same size");
        goto fail;
    }
    Py_INCREF(self->descr);
    values = (PyArrayObject *)\
        PyArray_FromAny(values0, self->descr, 0, 0, NPY_CARRAY, NULL);
    if (values == NULL) {
        goto fail;
    }
    nv = PyArray_SIZE(values); /* zero if null array */
    if (nv <= 0) {
        Py_XDECREF(values);
        Py_XDECREF(mask);
        Py_INCREF(Py_None);
        return Py_None;
    }
    if (PyDataType_REFCHK(self->descr)) {
        for (i = 0; i < ni; i++) {
            tmp = ((Bool *)(mask->data))[i];
            if (tmp) {
                src = values->data + chunk * (i % nv);
                PyArray_Item_INCREF(src, self->descr);
                PyArray_Item_XDECREF(dest+i*chunk, self->descr);
                memmove(dest + i * chunk, src, chunk);
            }
        }
    }
    else {
        func = self->descr->f->fastputmask;
        if (func == NULL) {
            for (i = 0; i < ni; i++) {
                tmp = ((Bool *)(mask->data))[i];
                if (tmp) {
                    src = values->data + chunk*(i % nv);
                    memmove(dest + i*chunk, src, chunk);
                }
            }
        }
        else {
            func(dest, mask->data, ni, values->data, nv);
        }
    }

    Py_XDECREF(values);
    Py_XDECREF(mask);
    if (copied) {
        Py_DECREF(self);
    }
    Py_INCREF(Py_None);
    return Py_None;

 fail:
    Py_XDECREF(mask);
    Py_XDECREF(values);
    if (copied) {
        PyArray_XDECREF_ERR(self);
    }
    return NULL;
}

thouis · 2012-10-19T20:44:07Z

@charris wrote on 2009-03-15

Where do you see the reference leak?

thouis · 2012-10-19T20:44:07Z

Attachment added by @cournape on 2009-03-15: memleak.patch

thouis · 2012-10-19T20:44:07Z

@cournape wrote on 2009-03-15

The leak appears only in the non-contiguous codepath (I attached the relevant path).

thouis · 2012-10-19T20:44:08Z

@cournape wrote on 2009-03-15

When I wanted to fix the alignment problem (on solaris), I added a condition to the PyArray_ISCONTIGUOUS (to enter this codepath in the non aligned case as well), and this codepath is buggy. It crashes on windows, and leaks on linux (with the patch, executing the test code shows a constant memory increase - I know that the descriptor is leaking, but I can't find the origin on it - and the current code does not look correct either).

thouis · 2012-10-19T20:44:08Z

@charris wrote on 2009-03-15

I think there is a fundamental problem here. The only array that can be modified in the array_putmask call is the one passed in. Yet in PyArray_PutMask we have

    {
        PyArrayObject *obj;
        int flags = NPY_CARRAY | NPY_UPDATEIFCOPY;

        Py_INCREF(self->descr);
        obj = (PyArrayObject *)PyArray_FromArray(self,
                                                 self->descr, flags);
        if (obj != self) {
            copied = 1;
        }
        self = obj;
    }

note that self is now a new array if copied == 1. But changes to this new self are purely local, the calling program will never see them. Even worse, the reference count of the new self will not be decremented on exit -> memory leak, while the reference count of the old self will be decremented on exit -> crash.

It looks to me like the only reasonable response with the current interface when copied == 1 is to raise an error. I think the function needs to be redesigned.

thouis · 2012-10-19T20:44:08Z

@charris wrote on 2009-03-15

Umm, nevermind. Blush. I didn't see what was going on with masks.

thouis · 2012-10-19T20:44:08Z

@charris wrote on 2009-03-16

OK, the refcount leak when the path is forced is because PyArray_FromArray always increments the refcount of the return whether or not a copy is made. With the path forced I see the following:

In [23]: a = eye(3)

In [24]: for i in range(10) :
    putmask(a,a,2)
    sys.getrefcount(a)
   ....: 
Out[26]: 3
Out[26]: 4
Out[26]: 5
Out[26]: 6
Out[26]: 7
Out[26]: 8
Out[26]: 9
Out[26]: 10
Out[26]: 11
Out[26]: 12

however, if I force a copy

In [32]: b = a[:2,:2]

In [33]: for i in range(10) :
    putmask(b,b,2)
    print sys.getrefcount(a), sys.getrefcount(b)
   ....: 
14 2
14 2
14 2
14 2
14 2
14 2
14 2
14 2
14 2
14 2

No problem. With the normal logic a copy is always forced so this error doesn't occur unless the contiguous flag is somehow bogus. In anycase, I don't think this is a problem in the current implementation, but copied = 1 should probably be set whenever that path is taken.

The PyArray_FastPutMask crash still needs to be figured out.

thouis · 2012-10-19T20:44:08Z

@mwiebe wrote on 2011-03-24

This appears to neither crash nor leak on Linux 64-bit with the 1.6 beta branch.

thouis · 2012-10-19T20:44:08Z

@rgommers wrote on 2011-03-25

Like David said on-list, working on linux is not a good reason to close this - it's a Windows issue. Tried this on win32, works fine with python 2.5. With python 3.1 I get the following with the test code in the first comment:

$ wine ../.wine/drive_c/Python31/python.exe crash.py 
Traceback (most recent call last):
  File "crash.py", line 12, in <module>
    base = ma.array(zip(ilist,flist,slist), mask=mask, dtype=ddtype)
  File "Z:\Users\zouzoujing\.wine\drive_c\Python31\lib\site-packages\numpy\ma\core.py", line 5735, in array
    fill_value=fill_value, ndmin=ndmin, shrink=shrink)
  File "Z:\Users\zouzoujing\.wine\drive_c\Python31\lib\site-packages\numpy\ma\core.py", line 2633, in __new__
    _data = np.array(data, dtype=dtype, copy=copy, subok=True, ndmin=ndmin)
TypeError: expected an object with a buffer interface

Not sure if that's related to the original crash or not.

thouis · 2012-10-19T20:44:09Z

Milestone changed to Unscheduled by @rgommers on 2011-03-25

thouis · 2012-10-19T20:44:10Z

@pv wrote on 2011-03-25

@ralf: the 3.1 issue is probably because unicode objects don't have the buffer interface -- try replacing the string literals with byte literals.

thouis · 2012-10-19T20:44:10Z

@rgommers wrote on 2011-03-25

Not sure how to specify the dtype to accomplish that?

slist = [b'one',b'two',b'three',b'four',b'five']
ddtype = [('a',int),('b',float),('c',??)]

thouis · 2012-10-19T20:44:10Z

@pv wrote on 2011-03-25

'S' dtype is still bytes also on Py3. But the real source of the "buffer interface" errors is that zip returns a generator on Py3 -- should be replaced with list(zip(...)).

thouis · 2012-10-19T20:44:10Z

@rgommers wrote on 2011-03-25

Okay, thanks. Works fine on Python 3.1 and 3.2 on win32 too.

thouis · 2012-10-19T20:44:11Z

Milestone changed to 1.6.0 by @mwiebe on 2011-05-31

thouis closed this as completed Oct 19, 2012

This was referenced Oct 23, 2012

masked array autotest fails with bus error (Trac #1094) #1692

Closed

Solaris10/SPARC/gcc-4.5.1/32bit: SIGBUS in test_mrecords.py (Trac #1631) #2227

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Crash on fastputmask on win32 (Trac #1032) #1630

Crash on fastputmask on win32 (Trac #1032) #1630

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Crash on fastputmask on win32 (Trac #1032) #1630

Crash on fastputmask on win32 (Trac #1032) #1630

Comments

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!