Commits

Lenard Lindstrom  committed db7a252

Fix surface locking problems; point save

Still work to be done with BufferProxy and Surface.get_buffer.

  • Participants
  • Parent commits 67f1c85

Comments (0)

Files changed (7)

File src/bufferproxy.c

         PyErr_NoMemory();
         return -1;
     }
+    pg_dict_view_p->consumer = pg_view_p->consumer;
     if (PgDict_AsBuffer(pg_dict_view_p, dict, flags)) {
         PyMem_Free(pg_dict_view_p);
         return -1;
         view_p = PyMem_New(Pg_buffer, 1);
         if (!view_p) {
             PyErr_NoMemory();
+            return 0;
         }
-        else if (proxy->get_buffer(proxy->obj, view_p, PyBUF_RECORDS)) {
+        view_p->consumer = (PyObject *)proxy;
+        if (proxy->get_buffer(proxy->obj, view_p, PyBUF_RECORDS)) {
             PyMem_Free(view_p);
-            view_p = 0;
+            return 0;
         }
-        else {
-            proxy->view_p = view_p;
-        }
+        proxy->view_p = view_p;
     }
     return (Py_buffer *)view_p;
 }
     }
 }
 
+static int
+_proxy_zombie_get_buffer(PyObject *obj, Pg_buffer *pg_view_p, int flags)
+{
+    PyObject *proxy = pg_view_p->consumer;
+
+    ((Py_buffer *)pg_view_p)->obj = 0;
+    PyErr_Format (PyExc_RuntimeError,
+                  "Attempted buffer export on <%s at %p, parent=<%s at %p>> "
+                  "while deallocating it",
+                  Py_TYPE(proxy)->tp_name, (void *)proxy,
+                  Py_TYPE(obj)->tp_name, (void *)obj);
+    return -1;
+}
+
 /**
  * Return a new PgBufproxyObject (Python level constructor).
  */
 
 /**
  * Deallocates the PgBufproxyObject and its members.
+ * Is reentrant.
  */
 static void
 proxy_dealloc(PgBufproxyObject *self)
 {
+    /* Prevent infinite recursion from a reentrant call */
+    if (self->get_buffer == _proxy_zombie_get_buffer) {
+        return;
+    }
+    self->get_buffer = _proxy_zombie_get_buffer;
+
+    /* Non reentrant call; deallocate */
     PyObject_GC_UnTrack(self);
     _proxy_release_view(self);
     Py_XDECREF(self->obj);
         PyErr_NoMemory();
         return -1;
     }
+    pg_obj_view_p->consumer = (PyObject *)self;
     if (self->get_buffer(self->obj, pg_obj_view_p, flags)) {
         PyMem_Free(pg_obj_view_p);
         return -1;

File src/pygame.h

 
 typedef struct pg_bufferinfo_s {
     Py_buffer view;
+    PyObject *consumer;                   /* Input: Borrowed reference */
     pg_releasebufferfunc release_buffer;
 } Pg_buffer;
 

File src/surface.c

     VIEWKIND_RAW
 } SurfViewKind;
 
+/* To avoid problems with non-const Py_buffer format field */
+static char FormatUint8[] = "B";
+static char FormatUint16[] = "=H";
+static char FormatUint32[] = "=I";
+
+typedef struct pg_bufferinternal_s {
+    PyObject *consumer_ref;    /* A weak reference to a bufferproxy object   */
+    Py_ssize_t mem[6];         /* Enough memory for dim 3 shape and strides  */
+} Pg_bufferinternal;
+
 int
 PySurface_Blit (PyObject * dstobj, PyObject * srcobj, SDL_Rect * dstrect,
                 SDL_Rect * srcrect, int the_args);
                                    int flags,
                                    char *name,
                                    Uint32 mask);
+static int _init_buffer(PyObject *surf, Pg_buffer *pg_view_p, int flags);
 static void _release_buffer(Py_buffer *view_p);
-static void _release_buffer_nomem(Py_buffer *view_p);
 static PyObject *_raise_get_view_ndim_error(int bitsize, SurfViewKind kind);
 
 static PyGetSetDef surface_getsets[] = {
     Py_buffer *view_p = (Py_buffer *)pg_view_p;
 
     view_p->obj = 0;
-    if (!PySurface_Lock (obj)) {
-        PyErr_SetString (PyExc_BufferError, "Unable to lock the surface");
+    if (_init_buffer (obj, pg_view_p, flags)) {
         return -1;
     }
     view_p->buf = surface->pixels;
     view_p->itemsize = 1;
     view_p->len = surface->pitch * surface->h;
     view_p->readonly = 0;
-    view_p->internal = 0;
-    view_p->format = (flags & PyBUF_FORMAT) ? "B" : 0;
-    view_p->shape = (flags & PyBUF_ND) ? &view_p->len : 0;
-    view_p->strides = (flags & PyBUF_STRIDES) ? &view_p->itemsize : 0;
-    view_p->suboffsets = 0;
-    view_p->internal = 0;
+    if (flags & PyBUF_FORMAT) {
+        view_p->format = FormatUint8;
+    }
+    if (flags & PyBUF_ND) {
+        view_p->shape[0] = view_p->len;
+    }
+    if (flags & PyBUF_STRIDES) {
+        view_p->strides[0] = view_p->itemsize;
+    }
     Py_INCREF (obj);
     view_p->obj = obj;
-    pg_view_p->release_buffer = _release_buffer_nomem;
     return 0;
 }
 
     Py_buffer *view_p = (Py_buffer *)pg_view_p;
     SDL_Surface *surface = PySurface_AsSurface (obj);
     Py_ssize_t itemsize = surface->format->BytesPerPixel;
+    char *format;
 
     view_p->obj = 0;
     if (itemsize == 1) {
     switch (itemsize) {
 
     case 2:
-        view_p->format = "=H";
+        format = FormatUint16;
         break;
     case 4:
-        view_p->format = "=I";
+        format = FormatUint32;
         break;
     default:
         /* Should not get here! */
                       (int)__LINE__, __FILE__, itemsize);
         return -1;
     }
-    if (!PySurface_Lock (obj)) {
-        PyErr_SetString (PyExc_BufferError, "Unable to lock the surface");
+    if (_init_buffer (obj, pg_view_p, flags)) {
         return -1;
     }
-    view_p->internal = (void *)(surface->w * surface->h);
+    view_p->format = format;
     view_p->buf = surface->pixels;
     view_p->itemsize = itemsize;
     view_p->ndim = 1;
     view_p->readonly = 0;
-    view_p->len = (Py_ssize_t)view_p->internal * itemsize;
+    view_p->len = surface->w * surface->h * itemsize;
     view_p->shape = (Py_ssize_t *)&view_p->internal;
     view_p->strides = (flags & PyBUF_STRIDES) ? &itemsize : 0;
     view_p->suboffsets = 0;
     Py_INCREF (obj);
     view_p->obj = obj;
-    pg_view_p->release_buffer = _release_buffer_nomem;
     return 0;
 }
 
     Py_buffer *view_p = (Py_buffer *)pg_view_p;
     SDL_Surface *surface = PySurface_AsSurface (obj);
     int itemsize = surface->format->BytesPerPixel;
+    char *format;
 
     view_p->obj = 0;
     if ((flags & PyBUF_RECORDS) != PyBUF_RECORDS) {
     switch (itemsize) {
 
     case 1:
-        view_p->format = "B";
+        format = FormatUint8;
         break;
     case 2:
-        view_p->format = "=H";
+        format = FormatUint16;
         break;
     case 4:
-        view_p->format = "=I";
+        format = FormatUint32;
         break;
     default:
         /* Should not get here! */
                       (int)__LINE__, __FILE__, itemsize);
         return -1;
     }
-    view_p->internal = PyMem_New (Py_ssize_t, 4);
-    if (!view_p->internal) {
-        PyErr_NoMemory ();
+    if (_init_buffer (obj, pg_view_p, flags)) {
         return -1;
     }
-    view_p->shape = (Py_ssize_t *)view_p->internal;
-    view_p->strides = view_p->shape + 2;
-    if (!PySurface_Lock (obj)) {
-        PyErr_SetString (PyExc_BufferError, "Unable to lock the surface");
-        PyMem_Free (view_p->internal);
-        return -1;
-    }
+    view_p->format = format;
     view_p->buf = surface->pixels;
     view_p->itemsize = itemsize;
     view_p->ndim = 2;
     view_p->suboffsets = 0;
     Py_INCREF (obj);
     view_p->obj = obj;
-    pg_view_p->release_buffer = _release_buffer;
     return 0;
 }
 
                          "A 3D surface view is not contiguous");
         return -1;
     }
-    view_p->internal = PyMem_New (Py_ssize_t, 6);
-    if (!view_p->internal) {
-        PyErr_NoMemory ();
+    if (_init_buffer (obj, pg_view_p, flags)) {
         return -1;
     }
-    view_p->shape = (Py_ssize_t *)view_p->internal;
-    view_p->strides = view_p->shape + 3;
-    if (!PySurface_Lock (obj)) {
-        PyErr_SetString (PyExc_BufferError, "Unable to lock the surface");
-        PyMem_Free (view_p->internal);
-        return -1;
-    }
-    view_p->format = "B";
+    view_p->format = FormatUint8;
     view_p->itemsize = 1;
     view_p->ndim = 3;
     view_p->readonly = 0;
     view_p->buf = startpixel;
     Py_INCREF (obj);
     view_p->obj = obj;
-    pg_view_p->release_buffer = _release_buffer;
     return 0;
 }
 
                       (int)__LINE__, __FILE__, (void *)mask);
         return -1;
     }
-    view_p->internal = PyMem_New (Py_ssize_t, 4);
-    if (!view_p->internal) {
-        PyErr_NoMemory ();
-        return -1;
-    }
-    view_p->shape = (Py_ssize_t *)view_p->internal;
-    view_p->strides = view_p->shape + 2;
-    if (!PySurface_Lock (obj)) {
-        PyErr_SetString (PyExc_BufferError, "Unable to lock the surface");
-        PyMem_Free (view_p->internal);
+    if (_init_buffer (obj, pg_view_p, flags)) {
         return -1;
     }
     view_p->buf = startpixel;
-    view_p->format = "B";
+    view_p->format = FormatUint8;
     view_p->itemsize = 1;
     view_p->ndim = 2;
     view_p->readonly = 0;
     view_p->strides[1] = surface->pitch;
     Py_INCREF (obj);
     view_p->obj = obj;
+    return 0;
+}
+
+static int
+_init_buffer (PyObject *surf, Pg_buffer *pg_view_p, int flags)
+{
+    PyObject *consumer;
+    Pg_bufferinternal *internal;
+
+/* A friendly self reminder */
+#warning To: len-l@telus.net, Msg: Please clean this up!
+    assert (surf &&
+            pg_view_p &&
+            PyObject_IsInstance (surf, (PyObject *)&PySurface_Type));
+    consumer = pg_view_p->consumer;
+    assert (consumer);
+    internal = PyMem_New (Pg_bufferinternal, 1);
+    if (!internal) {
+        PyErr_NoMemory ();
+        return -1;
+    }
+    internal->consumer_ref = PyWeakref_NewRef (consumer, 0);
+    if (!internal->consumer_ref) {
+        PyMem_Free (internal);
+        return -1;
+    }
+    if (!PySurface_LockBy (surf, consumer)) {
+        PyErr_Format (PyExc_BufferError,
+                      "Unable to lock <%s at %p> by <%s at %p>",
+                      Py_TYPE(surf)->tp_name, (void *)surf,
+                      Py_TYPE(consumer)->tp_name, (void *)consumer);
+        Py_DECREF (internal->consumer_ref);
+        PyMem_Free (internal);
+        return -1;
+    }
+    if ((flags & PyBUF_ND) == PyBUF_ND) {
+        ((Py_buffer *)pg_view_p)->shape = internal->mem;
+        if ((flags & PyBUF_STRIDES) == PyBUF_STRIDES) {
+            ((Py_buffer *)pg_view_p)->strides = internal->mem + 3;
+        }
+        else {
+            ((Py_buffer *)pg_view_p)->strides = 0;
+        }
+    }
+    else {
+        ((Py_buffer *)pg_view_p)->shape = 0;
+        ((Py_buffer *)pg_view_p)->strides = 0;
+    }
+    ((Py_buffer *)pg_view_p)->format = 0;
+    ((Py_buffer *)pg_view_p)->suboffsets = 0;
+    ((Py_buffer *)pg_view_p)->internal = internal;
     pg_view_p->release_buffer = _release_buffer;
     return 0;
 }
 static void
 _release_buffer (Py_buffer *view_p)
 {
-    PyMem_Free (view_p->internal);
-    PySurface_Unlock (view_p->obj);
-    Py_DECREF (view_p->obj);
-    view_p->obj = 0;
-}
-
-static void
-_release_buffer_nomem (Py_buffer *view_p)
-{
-    PySurface_Unlock (view_p->obj);
+    Pg_bufferinternal *internal;
+    PyObject *consumer_ref;
+    PyObject *consumer;
+
+    assert (view_p && view_p->obj && view_p->internal);
+    internal  = (Pg_bufferinternal *)view_p->internal;
+    consumer_ref = internal->consumer_ref;
+    assert (consumer_ref && PyWeakref_CheckRef (consumer_ref));
+    consumer = PyWeakref_GetObject (consumer_ref);
+    if (consumer) {
+        if (!PySurface_UnlockBy (view_p->obj, consumer)) {
+            PyErr_Clear ();
+        }
+    }
+    Py_DECREF (consumer_ref);
+    PyMem_Free (internal);
     Py_DECREF (view_p->obj);
     view_p->obj = 0;
 }

File test/base_test.py

 
         self.assert_( len(pygame.get_sdl_version()) == 3) 
 
+    class ExporterBase(object):
+        def __init__(self, shape, typechar, itemsize):
+            import ctypes
+
+            ndim = len(shape)
+            self.ndim = ndim
+            self.shape = tuple(shape)
+            array_len = 1
+            for d in shape:
+                array_len *= d
+            self.size = itemsize * array_len
+            self.parent = ctypes.create_string_buffer(self.size)
+            self.itemsize = itemsize
+            strides = [itemsize] * ndim
+            for i in range(ndim - 1, 0, -1):
+                strides[i - 1] = strides[i] * shape[i]
+            self.strides = tuple(strides)
+            self.data = ctypes.addressof(self.parent), False
+            if self.itemsize == 1:
+                byteorder = '|'
+            elif sys.byteorder == 'big':
+                byteorder = '>'
+            else:
+                byteorder = '<'
+            self.typestr = byteorder + typechar + str(self.itemsize)
+
+    def assertSame(self, proxy, obj):
+        self.assertEqual(proxy.length, obj.size)
+        d = proxy.__array_interface__
+        try:
+            self.assertEqual(d['typestr'], obj.typestr)
+            self.assertEqual(d['shape'], obj.shape)
+            self.assertEqual(d['strides'], obj.strides)
+            self.assertEqual(d['data'], obj.data)
+        finally:
+            d = None
+
+    def test_PgObject_GetBuffer_array_interface(self):
+        from pygame.bufferproxy import BufferProxy
+
+        class Exporter(self.ExporterBase):
+            def get__array_interface__(self):
+                return {'typestr': self.typestr,
+                        'shape': self.shape,
+                        'strides': self.strides,
+                        'data': self.data}
+            __array_interface__ = property(get__array_interface__)
+
+        _shape = [2, 3, 5, 7, 11]  # Some prime numbers
+        for ndim in range(1, len(_shape)):
+            o = Exporter(_shape[0:ndim], 'i', 2)
+            v = BufferProxy(o)
+            self.assertSame(v, o)
+        ndim = 2
+        shape = _shape[0:ndim]
+        for typechar in ('i', 'u'):
+            for itemsize in (1, 2, 4, 8):
+                o = Exporter(shape, typechar, itemsize)
+                v = BufferProxy(o)
+                self.assertSame(v, o)
+        for itemsize in (4, 8):
+            o = Exporter(shape, 'f', itemsize)
+            v = BufferProxy(o)
+            self.assertSame(v, o)
+        
+    def test_GetView_array_struct(self):
+        from pygame.bufferproxy import BufferProxy
+
+        class Exporter(self.ExporterBase):
+            def __init__(self, shape, typechar, itemsize):
+                super(Exporter, self).__init__(shape, typechar, itemsize)
+                self.view = BufferProxy(self.__dict__)
+
+            def get__array_struct__(self):
+                return self.view.__array_struct__
+            __array_struct__ = property(get__array_struct__)
+
+        _shape = [2, 3, 5, 7, 11]  # Some prime numbers
+        for ndim in range(1, len(_shape)):
+            o = Exporter(_shape[0:ndim], 'i', 2)
+            v = BufferProxy(o)
+            self.assertSame(v, o)
+        ndim = 2
+        shape = _shape[0:ndim]
+        for typechar in ('i', 'u'):
+            for itemsize in (1, 2, 4, 8):
+                o = Exporter(shape, typechar, itemsize)
+                v = BufferProxy(o)
+                self.assertSame(v, o)
+        for itemsize in (4, 8):
+            o = Exporter(shape, 'f', itemsize)
+            v = BufferProxy(o)
+            self.assertSame(v, o)
+
+    def test_GetView_newbuf(self):
+        self.fail()
+    if sys.version_info < (3, 0):
+        del test_GetView_newbuf
+        
     def not_init_assertions(self):
         self.assert_(not pygame.display.get_init(),
                      "display shouldn't be initialized" )

File test/bufferproxy_test.py

                      'data': (0, True),
                      'strides': (4, 20, 1)}
 
+    def test_module_name(self):
+        self.assertEqual(pygame.bufferproxy.__name__,
+                         "pygame.bufferproxy")
+
+    def test_class_name(self):
+        self.assertEqual(BufferProxy.__name__, "BufferProxy")
+
     def test___array_struct___property(self):
         kwds = self.view_keywords
         v = BufferProxy(kwds)
         self.assertEqual(len(success), 1)
         self.assertTrue(success[0])
 
+    def test_attribute(self):
+        v = BufferProxy(self.view_keywords)
+        self.assertRaises(AttributeError, getattr, v, 'undefined')
+        v.undefined = 12;
+        self.assertEqual(v.undefined, 12)
+        del v.undefined
+        self.assertRaises(AttributeError, getattr, v, 'undefined')
+
     def test_weakref(self):
         v = BufferProxy(self.view_keywords)
         weak_v = weakref.ref(v)
         class Obj(object):
             pass
         p = Obj()
+        a = Obj()
         r = [Obj(), Obj()]
         weak_p = weakref.ref(p)
+        weak_a = weakref.ref(a)
         weak_r0 = weakref.ref(r[0])
         weak_r1 = weakref.ref(r[1])
         weak_before = weakref.ref(before_callback)
         kwds['before'] = before_callback
         kwds['after'] = after_callback
         v = BufferProxy(kwds)
+        v.some_attribute = a
         weak_v = weakref.ref(v)
-        kwds = p = before_callback = after_callback = None
+        kwds = p = a = before_callback = after_callback = None
         gc.collect()
         self.assertTrue(weak_p() is not None)
+        self.assertTrue(weak_a() is not None)
         self.assertTrue(weak_before() is not None)
         self.assertTrue(weak_after() is not None)
         v = None
         gc.collect()
         self.assertTrue(weak_v() is None)
         self.assertTrue(weak_p() is None)
+        self.assertTrue(weak_a() is None)
         self.assertTrue(weak_before() is None)
         self.assertTrue(weak_after() is None)
         self.assertTrue(weak_r0() is not None)
         kwds = dict(self.view_keywords)
         kwds['parent'] = []
         v = BufferProxy(kwds)
+        v.some_attribute = v
         tracked = True
         for o in gc.get_objects():
             if o is v:
         self.assertEqual(r[:2], '*<')
         self.assertEqual(r[-2:], '>*')
 
+    def test_newbuf_arg(self):
+        from array import array
+
+        raw = as_bytes('abc\x00def')
+        b = array('B', raw)
+        b_address, b_nitems = b.buffer_info()
+        v = BufferProxy(b)
+        self.assertEqual(v.length, len(b))
+        self.assertEqual(v.raw, raw)
+        d = v.__array_interface__
+        try:
+            self.assertEqual(d['typestr'], '|u1')
+            self.assertEqual(d['shape'], (b_nitems,))
+            self.assertEqual(d['strides'], (b.itemsize,))
+            self.assertEqual(d['data'], (b_address, False))
+        finally:
+            d = None
+        b = array('h', [-1, 0, 2])
+        v = BufferProxy(b)
+        b_address, b_nitems = b.buffer_info()
+        b_nbytes = b.itemsize * b_nitems
+        self.assertEqual(v.length, b_nbytes)
+        try:
+            s = b.tostring()
+        except AttributeError:
+            s = b.tobytes()
+        self.assertEqual(v.raw, s)
+        d = v.__array_interface__
+        try:
+            lil_endian = pygame.get_sdl_byteorder() == pygame.LIL_ENDIAN
+            f = '{}i{}'.format('<' if lil_endian else '>', b.itemsize)
+            self.assertEqual(d['typestr'], f)
+            self.assertEqual(d['shape'], (b_nitems,))
+            self.assertEqual(d['strides'], (b.itemsize,))
+            self.assertEqual(d['data'], (b_address, False))
+        finally:
+            d = None
+    if sys.version_info < (3,):
+        del test_newbuf_arg
+
 class BufferProxyLegacyTest(unittest.TestCase):
     content = as_bytes('\x01\x00\x00\x02') * 12
     buffer = ctypes.create_string_buffer(content)

File test/mixer_test.py

         except ValueError:
             if not test_pass:
                 return
+            self.fail("Raised ValueError: Format %i, dtype %s" %
+                      (format, a.dtype))
         if not test_pass:
             self.fail("Did not raise ValueError: Format %i, dtype %s" %
                       (format, a.dtype))

File test/surface_test.py

         s = pygame.Surface((5, 7), 0, 24)
         self.assertRaises(Error, s.get_buffer, '0')
         self.assertRaises(Error, s.get_buffer, '1')
-        v = s.get_buffer('2')
-        self.assert_(isinstance(v, BufferProxy))
+        self.assertRaises(Error, s.get_buffer, '2')
         v = s.get_buffer('3')
         self.assert_(isinstance(v, BufferProxy))
 
         v = s.get_buffer('0')
         self.assert_(isinstance(v, BufferProxy))
         self.assertEqual(v.length, length)
-        v = s.get_buffer('1')
-        self.assert_(isinstance(v, BufferProxy))
-        self.assertEqual(v.length, length)
 
         s = pygame.Surface((5, 7), 0, 32)
         length = s.get_bytesize() * s.get_width() * s.get_height()
         # check the array interface structure fields.
         v = s.get_buffer('2')
         inter = ArrayInterface(v)
-        flags = PAI_FORTRAN | PAI_ALIGNED | PAI_NOTSWAPPED | PAI_WRITEABLE
+        flags = PAI_ALIGNED | PAI_NOTSWAPPED | PAI_WRITEABLE
         if (s.get_pitch() == s_w * s_bytesize):
-            flags |= PAI_CONTIGUOUS
+            flags |= PAI_FORTRAN
         self.assertEqual(inter.two, 2)
         self.assertEqual(inter.nd, 2)
         self.assertEqual(inter.typekind, 'u')
         # check the array interface structure fields.
         v = s.get_buffer('rgba'[plane])
         inter = ArrayInterface(v)
-        flags = PAI_ALIGNED | PAI_NOTSWAPPED | PAI_WRITEABLE | PAI_FORTRAN
+        flags = PAI_ALIGNED | PAI_NOTSWAPPED | PAI_WRITEABLE
         self.assertEqual(inter.two, 2)
         self.assertEqual(inter.nd, 2)
         self.assertEqual(inter.typekind, 'u')