BufferError when using memoryview

[coldeasy]

I noticed this when trying to use yappi with tornado. Here is a simple repro:

import yappi
yappi.start()


def test_mem():
    buf = bytearray()
    buf += b't' * 200
    view = memoryview(buf)[10:].tobytes()
    del buf[:10]
    return view


if __name__ == "__main__":
    test_mem()

Without yappi, the code completes successfully. With yappi I get the following error:

Traceback (most recent call last):
  File "yappi_repro.py", line 14, in <module>
    test_mem()
  File "yappi_repro.py", line 9, in test_mem
    del buf[:10]
BufferError: Existing exports of data: object cannot be re-sized

Reproducible on python 3.6, 3.7 and 3.8. I have not tried python 2.7

[sumerc]

Wow. This is pretty interesting :) I will be looking into this.

[sumerc]

The problem happens when we increment the reference count of the buildtin method of memoryview object in here:

yappi/yappi/_yappi.c

Line 575 in ba0a264

Py_INCREF(cfn);

I have no idea why this leads to the error described. Maybe anyone has any idea?

Note to future: Once we understand the root cause, I will fix it maybe just by remove support for function selector for builtin functions? Not sure.

[coldeasy]

I believe it's because we cannot refer to the memoryview while the underlying structure (bytearray) changes shape. When we increment the reference count of a view we can violate this constraint. A non-yappi demonstration of this:

# import yappi
# yappi.start()


def test_mem():
    buf = bytearray()
    buf += b't' * 200
    # Note tobytes() has been removed
    view = memoryview(buf)[10:]
    del buf[:10]
    return view


if __name__ == "__main__":
    test_mem()

Here the lifetime of the view exceeds the lifetime of the original buf structure so we get the same BufferError.

[sumerc]

Hmmm. I think you are right. Error message seemed a bit misleading, at least for me :) In fact we did not increment the view but one of the methods(tobytes() function) of the view but I assume that is same. We should not be touching the reference count of these kind of structures.

We increment the reference of every function being profiled in Yappi to support following convention:

def foo():
     pass

yappi.start()
foo()
stats = yappi.get_func_stats(
    filter_callback=lambda x: yappi.func_matches(x, [foo])
).print_all()

or in your case:

stats = yappi.get_func_stats(
    filter_callback=lambda x: yappi.func_matches(x, [memoryview.tobytes])
).print_all()

I am not sure about the fix, either.

Seeing these kind of issues, I am not sure if we include builtin functions for this kind of querying at all. I assume same kind of errors might happen as well for structures that hold references to other objects, somehow. Or maybe we could simply do not increment any ref. if the obj is memoryview.

Any idea on where this might break other than this situation?

[coldeasy]

The only thing that came to mind was items method on dictionaries but it looks like those views are dynamic and adapt when the dict is modified. I don't have any idea where else this issue might pop up, unfortunately.

I wonder is there some other method to implement the function equality checking. In your example we are checking equality against the class method - could we avoid supporting instance methods entirely and instead take a reference to the unbound method? Or is there a use-case where we need to keep instance method matching?

[sumerc]

I wonder is there some other method to implement the function equality checking. In your example we are checking equality against the class method - could we avoid supporting instance methods entirely and instead take a reference to the unbound method?

I did not look at that option. Worth looking into!

[sumerc]

Using method descriptors instead of PyCFunctionObject will prevent the possibility of selecting instance methods, but currently this does not seem like a big limitation.

And I think it is doable. I will fix for this on following days hopefully.

[coldeasy]

That's great, thanks for being so responsive. I'm happy to test your changes - currently I have this hack in place to unblock my progress

diff --git a/yappi/_yappi.c b/yappi/_yappi.c
index 7d21006..d8ea7f2 100644
--- a/yappi/_yappi.c
+++ b/yappi/_yappi.c
@@ -571,6 +571,11 @@ _ccode2pit(void *cco, uintptr_t current_tag)
         pit->builtin = 1;
         pit->modname = _pycfunction_module_name(cfn);
         pit->lineno = 0;
+       if (cfn->m_self && PyMemoryView_Check(cfn->m_self)) {
+           pit->fn_descriptor = PyStr_FromString("memoryview");
+           pit->name = PyStr_FromString("memoryview");
+           return pit;
+       }
         pit->fn_descriptor = (PyObject *)cfn;
         Py_INCREF(cfn);

[sumerc]

Hi again @coldeasy , I have managed to implement a fix for this issue. Could you please also verify on your end this works:
#61

Thanks!

[coldeasy]

Confirmed #61 fixes the issue, thanks @sumerc!