[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[pygame] [PLEASE TEST]: New locking code



Hi,

regarding Jake's unlock() report I changed the locking behaviour to be
more strict about who can unlock a locked surface.

While it was possible to completely unlock a surface at any time from
anywhere, it is now strongly tied to the object, which caused the lock.

While it was possible to do stuff like

      subsurface = surface.subsurface (...)
      subsurface.lock ()
      surface.unlock ()
      # surface is unlocked again although subsurface is locked
      ...
      sfarray = pygame.surfarray.pixels3d(surface)
      surface.unlock ()
      # Monsters live here
      ...

you now have to explicitly release the lock using the object that caused
it:

      subsurface = surface.subsurface (...)
      subsurface.unlock () 
      surface.unlock ()         # No effect
      subsurface.unlock ()      # Now the lock is released.
      # surface is unlocked again
      ...
      sfarray = pygame.surfarray.pixels3d(surface)
      surface.unlock ()         # Nothing happens
      del sfarray               # Surface will be unlocked
      # Rainbowland's coming!
      ...

The surface.get_locked() method was adjusted accordingly and works as
supposed now. Additionally the surface got a new get_locks() method,
which returns a tuple with the object holding a lock. Though this is not
always exact or very helpful, it can give you a quick overview about the
reason for a dangling lock (numpy surfarrays for example are not listed,
but a surface buffer instead as that one caused the lock).

As this is tightly bound to reference counts, weakref pointers and other
big sources for errors, I beg anyone to test it extensively with your
code.

Attached you'll find the patch, which hopefully brings love, joy,
 happiness and less errors to pygame.

Regards
Marcus
Index: src/pygame.h
===================================================================
--- src/pygame.h	(Revision 1297)
+++ src/pygame.h	(Arbeitskopie)
@@ -350,7 +350,8 @@
     SDL_Surface* surf;
     struct SubSurface_Data* subsurface;  /*ptr to subsurface data (if a
                                           * subsurface)*/
-    PyObject* weakreflist;
+    PyObject *weakreflist;
+    PyObject *locklist;
     PyObject *dependency;
 } PySurfaceObject;
 #define PySurface_AsSurface(x) (((PySurfaceObject*)x)->surf)
@@ -401,7 +402,7 @@
 /* SURFLOCK */    /*auto import/init by surface*/
 #define PYGAMEAPI_SURFLOCK_FIRSTSLOT                            \
     (PYGAMEAPI_SURFACE_FIRSTSLOT + PYGAMEAPI_SURFACE_NUMSLOTS)
-#define PYGAMEAPI_SURFLOCK_NUMSLOTS 5
+#define PYGAMEAPI_SURFLOCK_NUMSLOTS 8
 struct SubSurface_Data
 {
     PyObject* owner;
@@ -409,23 +410,41 @@
     int offsetx, offsety;
 };
 
+typedef struct
+{
+    PyObject_HEAD
+    PyObject *surface;
+    PyObject *lockobj;
+    PyObject *weakrefs;
+} PyLifetimeLock;
+
 #ifndef PYGAMEAPI_SURFLOCK_INTERNAL
+#define PyLifetimeLock_Check(x)                         \
+    ((x)->ob_type == (PyTypeObject*)                    \
+        PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 0])
 #define PySurface_Prep(x)                                               \
     if(((PySurfaceObject*)x)->subsurface)                               \
         (*(*(void(*)(PyObject*))                                        \
-           PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 0]))(x)
+           PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 1]))(x)
 
 #define PySurface_Unprep(x)                                             \
     if(((PySurfaceObject*)x)->subsurface)                               \
         (*(*(void(*)(PyObject*))                                        \
-           PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 1]))(x)
+           PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 2]))(x)
 
 #define PySurface_Lock                                                  \
-    (*(int(*)(PyObject*))PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 2])
+    (*(int(*)(PyObject*))PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 3])
 #define PySurface_Unlock                                                \
-    (*(int(*)(PyObject*))PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 3])
+    (*(int(*)(PyObject*))PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 4])
+#define PySurface_LockBy                                                \
+    (*(int(*)(PyObject*,PyObject*))                                     \
+        PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 5])
+#define PySurface_UnlockBy                                              \
+    (*(int(*)(PyObject*,PyObject*))                                     \
+        PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 6])
 #define PySurface_LockLifetime                                          \
-    (*(PyObject*(*)(PyObject*))PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 4])
+    (*(PyObject*(*)(PyObject*,PyObject*))                               \
+        PyGAME_C_API[PYGAMEAPI_SURFLOCK_FIRSTSLOT + 7])
 #endif
 
 
@@ -501,6 +520,18 @@
 #endif
 
 /* BufferProxy */
+typedef struct
+{
+    PyObject_HEAD
+    PyObject *dict;     /* dict for subclassing */
+    PyObject *weakrefs; /* Weakrefs for subclassing */
+    void *buffer;       /* Pointer to the buffer of the parent object. */
+    Py_ssize_t length;  /* Length of the buffer. */
+    PyObject *parent;   /* Parent object associated with this object. */
+    PyObject *lock;     /* Lock object for the surface. */
+
+} PyBufferProxy;
+
 #define PYGAMEAPI_BUFFERPROXY_FIRSTSLOT                                 \
     (PYGAMEAPI_RWOBJECT_FIRSTSLOT + PYGAMEAPI_RWOBJECT_NUMSLOTS)
 #define PYGAMEAPI_BUFFERPROXY_NUMSLOTS 2
Index: src/pygamedocs.h
===================================================================
--- src/pygamedocs.h	(Revision 1297)
+++ src/pygamedocs.h	(Arbeitskopie)
@@ -717,6 +717,8 @@
 
 #define DOC_SURFACEGETLOCKED "Surface.get_locked(): return bool\ntest if the Surface is current locked"
 
+#define DOC_SURFACEGETLOCKS "Surface.get_locks(): return tuple\nGets the locks from the Surface"
+
 #define DOC_SURFACEGETAT "Surface.get_at((x, y)): return Color\nget the color value at a single pixel"
 
 #define DOC_SURFACESETAT "Surface.set_at((x, y), Color): return None\nset the color value for a single pixel"
Index: src/surface.doc
===================================================================
--- src/surface.doc	(Revision 1297)
+++ src/surface.doc	(Arbeitskopie)
@@ -311,6 +311,14 @@
 <END>
 
 
+get_locks
+Gets the locks from the Surface
+Surface.get_locks(): return tuple
+
+Returns the currently existing locks for the Surface.
+<END>
+
+
 get_at
 get the color value at a single pixel
 Surface.get_at((x, y)): return Color
Index: src/surface.c
===================================================================
--- src/surface.c	(Revision 1297)
+++ src/surface.c	(Arbeitskopie)
@@ -49,6 +49,7 @@
 static PyObject *surf_unlock (PyObject *self);
 static PyObject *surf_mustlock (PyObject *self);
 static PyObject *surf_get_locked (PyObject *self);
+static PyObject *surf_get_locks (PyObject *self);
 static PyObject *surf_get_palette (PyObject *self);
 static PyObject *surf_get_palette_at (PyObject *self, PyObject *args);
 static PyObject *surf_set_palette (PyObject *self, PyObject *args);
@@ -106,6 +107,8 @@
       DOC_SURFACEMUSTLOCK },
     { "get_locked", (PyCFunction) surf_get_locked, METH_NOARGS,
       DOC_SURFACEGETLOCKED },
+    { "get_locks", (PyCFunction) surf_get_locks, METH_NOARGS,
+      DOC_SURFACEGETLOCKS },
 
     { "set_colorkey", surf_set_colorkey, METH_VARARGS, DOC_SURFACESETCOLORKEY },
     { "get_colorkey", (PyCFunction) surf_get_colorkey, METH_NOARGS,
@@ -239,6 +242,8 @@
         self->surf = NULL;
         self->subsurface = NULL;
         self->weakreflist = NULL;
+        self->dependency = NULL;
+        self->locklist = NULL;
     }
     return (PyObject *) self;
 }
@@ -269,6 +274,12 @@
         Py_DECREF (self->dependency);
         self->dependency = NULL;
     }
+
+    if (self->locklist)
+    {
+        Py_DECREF (self->locklist);
+        self->locklist = NULL;
+    }
 }
 
 static void 
@@ -667,8 +678,7 @@
 static PyObject*
 surf_unlock (PyObject *self)
 {
-    if (!PySurface_Unlock (self))
-        return NULL;
+    PySurface_Unlock (self);
     Py_RETURN_NONE;
 }
 
@@ -677,19 +687,43 @@
 {
     SDL_Surface *surf = PySurface_AsSurface (self);
     return PyInt_FromLong (SDL_MUSTLOCK (surf) ||
-                           ((PySurfaceObject *) self)->subsurface);
+        ((PySurfaceObject *) self)->subsurface);
 }
 
 static PyObject*
 surf_get_locked (PyObject *self)
 {
     PySurfaceObject *surf = (PySurfaceObject *) self;
-    if (surf->surf->pixels != NULL)
+
+    if (surf->locklist && PyList_Size (surf->locklist) > 0)
         Py_RETURN_TRUE;
     Py_RETURN_FALSE;
 }
 
 static PyObject*
+surf_get_locks (PyObject *self)
+{
+    PySurfaceObject *surf = (PySurfaceObject *) self;
+    Py_ssize_t len, i = 0;
+    PyObject *tuple, *tmp;
+    if (!surf->locklist)
+        return PyTuple_New (0);
+
+    len = PyList_Size (surf->locklist);
+    tuple = PyTuple_New (len);
+    if (!tuple)
+        return NULL;
+    
+    for (i = 0; i < len; i++)
+    {
+        tmp = PyWeakref_GetObject (PyList_GetItem (surf->locklist, i));
+        Py_INCREF (tmp);
+        PyTuple_SetItem (tuple, i, tmp);
+    }
+    return tuple;
+}
+
+static PyObject*
 surf_get_palette (PyObject *self)
 {
     SDL_Surface *surf = PySurface_AsSurface (self);
@@ -1797,23 +1831,26 @@
     Py_ssize_t length;
 
     length = (Py_ssize_t) surface->pitch * surface->h;
-    lock = PySurface_LockLifetime (self);
+
+    buffer = PyBufferProxy_New (self, NULL, length, NULL);
+    if (!buffer)
+    {
+        return RAISE (PyExc_SDLError,
+            "could not acquire a buffer for the surface");
+    }
+    
+    lock = PySurface_LockLifetime (self, buffer);
     if (!lock)
     {
+        Py_DECREF (buffer);
         return RAISE (PyExc_SDLError, "could not lock surface");
     }
+    ((PyBufferProxy *) buffer)->buffer = surface->pixels;
+    ((PyBufferProxy *) buffer)->lock = lock;
 
-    buffer = PyBufferProxy_New (self, surface->pixels, length, lock);
-    if (!buffer)
-    {
-        Py_DECREF (lock);
-        return RAISE (PyExc_SDLError,
-                      "could not acquire a buffer for the surface");
-    }
     return buffer;
 }
 
-
 /*this internal blit function is accessable through the C api*/
 int 
 PySurface_Blit (PyObject * dstobj, PyObject * srcobj, SDL_Rect * dstrect,
Index: src/surflock.c
===================================================================
--- src/surflock.c	(Revision 1297)
+++ src/surflock.c	(Arbeitskopie)
@@ -1,6 +1,7 @@
 /*
   pygame - Python Game Library
   Copyright (C) 2000-2001  Pete Shinners
+  Copyright (C) 2008 Marcus von Appen
 
   This library is free software; you can redistribute it and/or
   modify it under the terms of the GNU Library General Public
@@ -28,7 +29,11 @@
 
 static int PySurface_Lock (PyObject* surfobj);
 static int PySurface_Unlock (PyObject* surfobj);
+static int PySurface_LockBy (PyObject* surfobj, PyObject* lockobj);
+static int PySurface_UnlockBy (PyObject* surfobj, PyObject* lockobj);
 
+static void _lifelock_dealloc (PyObject* self);
+
 static void
 PySurface_Prep (PyObject* surfobj)
 {
@@ -37,7 +42,7 @@
     {
         SDL_Surface* surf = PySurface_AsSurface (surfobj);
         SDL_Surface* owner = PySurface_AsSurface (data->owner);
-        PySurface_Lock (data->owner);
+        PySurface_LockBy (data->owner, surfobj);
         surf->pixels = ((char*) owner->pixels) + data->pixeloffset;
     }
 }
@@ -47,13 +52,43 @@
 {
     struct SubSurface_Data* data = ((PySurfaceObject*) surfobj)->subsurface;
     if (data)
-        PySurface_Unlock (data->owner);
+        PySurface_UnlockBy (data->owner, surfobj);
 }
 
 static int
 PySurface_Lock (PyObject* surfobj)
 {
+    return PySurface_LockBy (surfobj, surfobj);
+}
+
+static int
+PySurface_Unlock (PyObject* surfobj)
+{
+    return PySurface_UnlockBy (surfobj, surfobj);
+}
+
+static int
+PySurface_LockBy (PyObject* surfobj, PyObject* lockobj)
+{
+    PyObject *ref;
     PySurfaceObject* surf = (PySurfaceObject*) surfobj;
+
+    if (!surf->locklist)
+    {
+        surf->locklist = PyList_New (0);
+        if (!surf->locklist)
+            return 0;
+    }
+    ref = PyWeakref_NewRef (lockobj, NULL);
+    if (!ref)
+        return 0;
+    if (ref == Py_None)
+    {
+        Py_DECREF (ref);
+        return 0;
+    }
+    PyList_Append (surf->locklist, ref);
+
     if (surf->subsurface)
         PySurface_Prep (surfobj);
     if (SDL_LockSurface (surf->surf) == -1)
@@ -65,59 +100,143 @@
 }
 
 static int
-PySurface_Unlock (PyObject* surfobj)
+PySurface_UnlockBy (PyObject* surfobj, PyObject* lockobj)
 {
     PySurfaceObject* surf = (PySurfaceObject*) surfobj;
-    if (surf->surf != NULL)
-        SDL_UnlockSurface (surf->surf);
-    if (surf->subsurface)
-        PySurface_Unprep (surfobj);
-    return 1;
+    int found = 0;
+    int noerror = 1;
+
+    if (surf->locklist)
+    {
+        PyObject *item, *ref;
+        Py_ssize_t len = PyList_Size (surf->locklist);
+        while (--len >= 0 && !found)
+        {
+            item = PyList_GetItem (surf->locklist, len);
+            ref = PyWeakref_GetObject (item);
+            if (ref == lockobj)
+            {
+                if (PySequence_DelItem (surf->locklist, len) == -1)
+                    return 0;
+                else
+                    found = 1;
+            }
+        }
+
+        /* Clear dead references */
+        len = PyList_Size (surf->locklist);
+        while (--len >= 0)
+        {
+            item = PyList_GetItem (surf->locklist, len);
+            ref = PyWeakref_GetObject (item);
+            if (ref == Py_None)
+            {
+                if (PySequence_DelItem (surf->locklist, len) == -1)
+                    noerror = 0;
+                else
+                    found++;
+            }
+        }
+    }
+
+    if (!found)
+        return noerror;
+
+    /* Release all found locks. */
+    while (found > 0)
+    {
+        if (surf->surf != NULL)
+            SDL_UnlockSurface (surf->surf);
+        if (surf->subsurface)
+            PySurface_Unprep (surfobj);
+        found--;
+    }
+
+    return noerror;
 }
 
-/* lifetimelock object internals */
-typedef struct
+
+static PyTypeObject PyLifetimeLock_Type =
 {
-    PyObject_HEAD
-    PyObject* surface;
-} PyLifetimeLockObject;
+    PyObject_HEAD_INIT(NULL)
+    0,                          /*size*/
+    "SurfLifeLock",             /*name*/
+    sizeof(PyLifetimeLock),     /*basic size*/
+    0,                          /* tp_itemsize */
+    _lifelock_dealloc,          /* tp_dealloc*/
+    0,                          /* tp_print */
+    0,                          /* tp_getattr */
+    0,                          /* tp_setattr */
+    0,                          /* tp_compare */
+    0,                          /* tp_repr */
+    0,                          /* tp_as_number */
+    0,                          /* tp_as_sequence */
+    0,                          /* tp_as_mapping */
+    0,                          /* tp_hash */
+    0,                          /* tp_call */
+    0,                          /* tp_str */
+    0,                          /* tp_getattro */
+    0,                          /* tp_setattro */
+    0,                          /* tp_as_buffer */
+    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_WEAKREFS,
+    0,                          /* tp_doc */
+    0,                          /* tp_traverse */
+    0,                          /* tp_clear */
+    0,                          /* tp_richcompare */
+    offsetof (PyLifetimeLock, weakrefs),  /* tp_weaklistoffset */
+    0,                          /* tp_iter */
+    0,                          /* tp_iternext */
+    0,                          /* tp_methods */
+    0,                          /* tp_members */
+    0,                          /* tp_getset */
+    0,                          /* tp_base */
+    0,                          /* tp_dict */
+    0,                          /* tp_descr_get */
+    0,                          /* tp_descr_set */
+    0,                          /* tp_dictoffset */
+    0,                          /* tp_init */
+    0,                          /* tp_alloc */
+    0,                          /* tp_new */
+    0,                          /* tp_free */
+    0,                          /* tp_is_gc */
+    0,                          /* tp_bases */
+    0,                          /* tp_mro */
+    0,                          /* tp_cache */
+    0,                          /* tp_subclasses */
+    0,                          /* tp_weaklist */
+    0                           /* tp_del */
+};
 
+/* lifetimelock object internals */
 static void
-lifelock_dealloc (PyObject* self)
+_lifelock_dealloc (PyObject* self)
 {
-    PyLifetimeLockObject* lifelock = (PyLifetimeLockObject*) self;
-    
-    PySurface_Unlock (lifelock->surface);
+    PyLifetimeLock* lifelock = (PyLifetimeLock*) self;
+
+    if (lifelock->weakrefs)
+        PyObject_ClearWeakRefs (self);
+
+    PySurface_UnlockBy (lifelock->surface, lifelock->lockobj);
     Py_DECREF (lifelock->surface);
-    
     PyObject_DEL (self);
 }
 
-static PyTypeObject PyLifetimeLock_Type =
-{
-    PyObject_HEAD_INIT(NULL)
-    0,                           /*size*/
-    "SurfLifeLock",              /*name*/
-    sizeof(PyLifetimeLockObject),/*basic size*/
-    0,                           /*itemsize*/
-    lifelock_dealloc,            /*dealloc*/
-};
-
 static PyObject*
-PySurface_LockLifetime (PyObject* surf)
+PySurface_LockLifetime (PyObject* surfobj, PyObject *lockobj)
 {
-    PyLifetimeLockObject* life;
-    if (!surf)
+    PyLifetimeLock* life;
+    if (!surfobj)
         return RAISE (PyExc_SDLError, SDL_GetError ());
     
-    if (!PySurface_Lock (surf))
-        return NULL;
-    
-    life = PyObject_NEW (PyLifetimeLockObject, &PyLifetimeLock_Type);
+    life = PyObject_NEW (PyLifetimeLock, &PyLifetimeLock_Type);
     if (life)
     {
-        life->surface = surf;
-        Py_INCREF (surf);
+        life->surface = surfobj;
+        life->lockobj = lockobj;
+        life->weakrefs = NULL;
+        Py_INCREF (surfobj);
+        if (!PySurface_LockBy (surfobj, lockobj))
+            return NULL;
     }
     return (PyObject*) life;
 }
@@ -141,11 +260,14 @@
     dict = PyModule_GetDict (module);
     
     /* export the c api */
-    c_api[0] = PySurface_Prep;
-    c_api[1] = PySurface_Unprep;
-    c_api[2] = PySurface_Lock;
-    c_api[3] = PySurface_Unlock;
-    c_api[4] = PySurface_LockLifetime;
+    c_api[0] = &PyLifetimeLock_Type;
+    c_api[1] = PySurface_Prep;
+    c_api[2] = PySurface_Unprep;
+    c_api[3] = PySurface_Lock;
+    c_api[4] = PySurface_Unlock;
+    c_api[5] = PySurface_LockBy;
+    c_api[6] = PySurface_UnlockBy;
+    c_api[7] = PySurface_LockLifetime;
     apiobj = PyCObject_FromVoidPtr (c_api, NULL);
     PyDict_SetItemString (dict, PYGAMEAPI_LOCAL_ENTRY, apiobj);
     Py_DECREF (apiobj);
Index: src/_numericsurfarray.c
===================================================================
--- src/_numericsurfarray.c	(Revision 1297)
+++ src/_numericsurfarray.c	(Arbeitskopie)
@@ -73,7 +73,7 @@
     array = PyArray_FromDimsAndData (3, dim, PyArray_UBYTE, "");
     if (array)
     {
-        lifelock = PySurface_LockLifetime (surfobj);
+        lifelock = PySurface_LockLifetime (surfobj, array);
         if (!lifelock)
         {
             Py_DECREF (array);
@@ -122,7 +122,7 @@
     array = PyArray_FromDimsAndData (2, dim, type, "");
     if (array)
     {
-        lifelock = PySurface_LockLifetime (surfobj);
+        lifelock = PySurface_LockLifetime (surfobj, array);
         if (!lifelock)
         {
             Py_DECREF (array);
@@ -170,7 +170,7 @@
     array = PyArray_FromDimsAndData (2, dim, PyArray_UBYTE, "");
     if(array)
     {
-        lifelock = PySurface_LockLifetime (surfobj);
+        lifelock = PySurface_LockLifetime (surfobj, array);
         if (!lifelock)
         {
             Py_DECREF (array);
@@ -212,7 +212,7 @@
     stridex = ((PyArrayObject*) array)->strides[0];
     stridey = ((PyArrayObject*) array)->strides[1];
 
-    if (!PySurface_Lock (surfobj))
+    if (!PySurface_LockBy (surfobj, array))
     {
         Py_DECREF (array);
         return NULL;
@@ -279,7 +279,7 @@
         break;
     }
     
-    if (!PySurface_Unlock (surfobj))
+    if (!PySurface_UnlockBy (surfobj, array))
     {
         Py_DECREF (array);
         return NULL;
@@ -328,7 +328,7 @@
     stridex = ((PyArrayObject*) array)->strides[0];
     stridey = ((PyArrayObject*) array)->strides[1];
 
-    if (!PySurface_Lock (surfobj))
+    if (!PySurface_LockBy (surfobj, array))
     {
         Py_DECREF (array);
         return NULL;
@@ -340,7 +340,7 @@
         if (!format->palette)
         {
             Py_DECREF (array);
-            if (!PySurface_Unlock (surfobj))
+            if (!PySurface_UnlockBy (surfobj, array))
                 return NULL;
             return RAISE (PyExc_RuntimeError, "8bit surface has no palette");
         }
@@ -420,7 +420,7 @@
         break;
     }
     
-    if (!PySurface_Unlock (surfobj))
+    if (!PySurface_UnlockBy (surfobj, array))
     {
         Py_DECREF (array);
         return NULL;
@@ -467,7 +467,7 @@
     stridex = ((PyArrayObject*) array)->strides[0];
     stridey = ((PyArrayObject*) array)->strides[1];
     
-    if (!PySurface_Lock (surfobj))
+    if (!PySurface_LockBy (surfobj, array))
     {
         Py_DECREF (array);
         return NULL;
@@ -525,7 +525,7 @@
         break;
     }
     
-    if (!PySurface_Unlock (surfobj))
+    if (!PySurface_UnlockBy (surfobj, array))
     {
         Py_DECREF (array);
         return NULL;
@@ -569,7 +569,7 @@
     stridex = ((PyArrayObject*) array)->strides[0];
     stridey = ((PyArrayObject*) array)->strides[1];
     
-    if (!PySurface_Lock (surfobj))
+    if (!PySurface_LockBy (surfobj, array))
     {
         Py_DECREF (array);
         return NULL;
@@ -640,7 +640,7 @@
         break;
     }
     
-    if (!PySurface_Unlock (surfobj))
+    if (!PySurface_UnlockBy (surfobj, array))
     {
         Py_DECREF (array);
         return NULL;
@@ -870,7 +870,7 @@
     
     if (sizex != surf->w || sizey != surf->h)
         return RAISE (PyExc_ValueError, "array must match surface dimensions");
-    if (!PySurface_Lock (surfobj))
+    if (!PySurface_LockBy (surfobj, (PyObject *) array))
         return NULL;
     
     switch (surf->format->BytesPerPixel)
@@ -893,7 +893,7 @@
                 COPYMACRO_2D(Uint8, Uint64);
                 break;
             default:
-                if (!PySurface_Unlock (surfobj))
+                if (!PySurface_UnlockBy (surfobj, (PyObject *) array))
                     return NULL;
                 return RAISE (PyExc_ValueError,
                               "unsupported datatype for array\n");
@@ -918,7 +918,7 @@
                 COPYMACRO_2D(Uint16, Uint64);
                 break;
             default:
-                if (!PySurface_Unlock (surfobj))
+                if (!PySurface_UnlockBy (surfobj, (PyObject *) array))
                     return NULL;
                 return RAISE (PyExc_ValueError,
                               "unsupported datatype for array\n");
@@ -941,7 +941,7 @@
                 COPYMACRO_3D(Uint16, Uint64);
                 break;
             default:
-                if (!PySurface_Unlock (surfobj))
+                if (!PySurface_UnlockBy (surfobj, (PyObject *) array))
                     return NULL;
                 return RAISE (PyExc_ValueError,
                               "unsupported datatype for array\n");
@@ -966,7 +966,7 @@
                 COPYMACRO_2D_24(Uint64);
                 break;
             default:
-                if (!PySurface_Unlock (surfobj))
+                if (!PySurface_UnlockBy (surfobj, (PyObject *) array))
                     return NULL;
                 return RAISE (PyExc_ValueError,
                               "unsupported datatype for array\n");
@@ -989,7 +989,7 @@
                 COPYMACRO_3D_24(Uint64);
                 break;
             default:
-                if (!PySurface_Unlock (surfobj))
+                if (!PySurface_UnlockBy (surfobj, (PyObject *) array))
                     return NULL;
                 return RAISE (PyExc_ValueError,
                               "unsupported datatype for array\n");
@@ -1014,7 +1014,7 @@
                 COPYMACRO_2D(Uint32, Uint64);
                 break;
             default:
-                if (!PySurface_Unlock (surfobj))
+                if (!PySurface_UnlockBy (surfobj, (PyObject *) array))
                     return NULL;
                 return RAISE (PyExc_ValueError,
                              "unsupported datatype for array\n");
@@ -1037,7 +1037,7 @@
                 COPYMACRO_3D(Uint32, Uint64);
                 break;
             default:
-                if (!PySurface_Unlock (surfobj))
+                if (!PySurface_UnlockBy (surfobj, (PyObject *) array))
                     return NULL;
                 return RAISE (PyExc_ValueError,
                               "unsupported datatype for array\n");
@@ -1045,12 +1045,12 @@
         }
         break;
     default:
-        if (!PySurface_Unlock (surfobj))
+        if (!PySurface_UnlockBy (surfobj, (PyObject *) array))
             return NULL;
         return RAISE (PyExc_RuntimeError, "unsupported bit depth for image");
     }
     
-    if (!PySurface_Unlock (surfobj))
+    if (!PySurface_UnlockBy (surfobj, (PyObject *) array))
         return NULL;
     Py_RETURN_NONE;
 }
Index: src/bufferproxy.c
===================================================================
--- src/bufferproxy.c	(Revision 1297)
+++ src/bufferproxy.c	(Arbeitskopie)
@@ -23,18 +23,6 @@
 #include "pygame.h"
 #include "pygamedocs.h"
 
-typedef struct
-{
-    PyObject_HEAD
-    PyObject *dict;     /* dict for subclassing */
-    PyObject *weakrefs; /* Weakrefs for subclassing */
-    void *buffer;       /* Pointer to the buffer of the parent object. */
-    Py_ssize_t length;  /* Length of the buffer. */
-    PyObject *parent;   /* Parent object associated with this object. */
-    PyObject *lock;     /* Lock object for the surface. */
-
-} PyBufferProxy;
-
 static PyObject* _bufferproxy_new (PyTypeObject *type, PyObject *args,
                                    PyObject *kwds);
 static void _bufferproxy_dealloc (PyBufferProxy *self);
@@ -178,10 +166,8 @@
 {
     if (self->weakrefs)
         PyObject_ClearWeakRefs ((PyObject *) self);
-    if (self->lock)
-    {
-        Py_DECREF (self->lock);
-    }
+
+    Py_XDECREF (self->lock);
     Py_XDECREF (self->dict);
     self->ob_type->tp_free ((PyObject *) self);
 }
Index: src/pixelarray.c
===================================================================
--- src/pixelarray.c	(Revision 1297)
+++ src/pixelarray.c	(Arbeitskopie)
@@ -123,7 +123,7 @@
 {
     { "__dict__", (getter) _pxarray_get_dict, NULL, NULL, NULL },
     { "surface", (getter) _pxarray_get_surface, NULL, DOC_PIXELARRAYSURFACE,
-      NULL },
+      NULL }, 
     { NULL, NULL, NULL, NULL, NULL }
 };
 
@@ -226,7 +226,7 @@
         /* Initial PixelArray */
         if (surface)
         {
-            self->lock = PySurface_LockLifetime (surface);
+            self->lock = PySurface_LockLifetime (surface, (PyObject *) self);
             if (!self->lock)
             {
                 Py_DECREF (surface);
@@ -239,7 +239,7 @@
     {
         self->parent = parent;
         Py_INCREF (parent);
-        self->lock = ((PyPixelArray*) parent)->lock;
+        self->lock = ((PyPixelArray *)parent)->lock;
         Py_INCREF (self->lock);
     }
 
Index: docs/ref/surface.html
===================================================================
--- docs/ref/surface.html	(Revision 1297)
+++ docs/ref/surface.html	(Arbeitskopie)
@@ -63,6 +63,7 @@
   <tr><td><a href="surface.html#Surface.unlock">Surface.unlock</a> - <font size=-1>unlock the Surface memory from pixel access</font></td><td>unlock the Surface memory from pixel access</td></tr>
   <tr><td><a href="surface.html#Surface.mustlock">Surface.mustlock</a> - <font size=-1>test if the Surface requires locking</font></td><td>test if the Surface requires locking</td></tr>
   <tr><td><a href="surface.html#Surface.get_locked">Surface.get_locked</a> - <font size=-1>test if the Surface is current locked</font></td><td>test if the Surface is current locked</td></tr>
+  <tr><td><a href="surface.html#Surface.get_locks">Surface.get_locks</a> - <font size=-1>Gets the locks from the Surface</font></td><td>Gets the locks from the Surface</td></tr>
   <tr><td><a href="surface.html#Surface.get_at">Surface.get_at</a> - <font size=-1>get the color value at a single pixel</font></td><td>get the color value at a single pixel</td></tr>
   <tr><td><a href="surface.html#Surface.set_at">Surface.set_at</a> - <font size=-1>set the color value for a single pixel</font></td><td>set the color value for a single pixel</td></tr>
   <tr><td><a href="surface.html#Surface.get_palette">Surface.get_palette</a> - <font size=-1>get the color index palette for an 8bit Surface</font></td><td>get the color index palette for an 8bit Surface</td></tr>
@@ -256,6 +257,15 @@
 <br></ul>
 
 
+<a name="Surface.get_locks">
+<big><b>Surface.get_locks</big></b><br><ul>
+  <i>Gets the locks from the Surface</i><br>
+  <tt>Surface.get_locks(): return tuple</tt><br>
+<p>Returns the currently existing locks for the Surface. </p>
+<!--COMMENTS:Surface.get_locks--> &nbsp;<br> 
+<br></ul>
+
+
 <a name="Surface.get_at">
 <big><b>Surface.get_at</big></b><br><ul>
   <i>get the color value at a single pixel</i><br>
Index: docs/ref/index.html
===================================================================
--- docs/ref/index.html	(Revision 1297)
+++ docs/ref/index.html	(Arbeitskopie)
@@ -473,6 +473,7 @@
 <li><a href="surface.html#Surface.get_flags">Surface.get_flags</a> - <font size=-1>get the additional flags used for the Surface</font></li>
 <li><a href="surface.html#Surface.get_height">Surface.get_height</a> - <font size=-1>get the height of the Surface</font></li>
 <li><a href="surface.html#Surface.get_locked">Surface.get_locked</a> - <font size=-1>test if the Surface is current locked</font></li>
+<li><a href="surface.html#Surface.get_locks">Surface.get_locks</a> - <font size=-1>Gets the locks from the Surface</font></li>
 <li><a href="surface.html#Surface.get_losses">Surface.get_losses</a> - <font size=-1>the significant bits used to convert between a color and a mapped integer</font></li>
 <li><a href="surface.html#Surface.get_masks">Surface.get_masks</a> - <font size=-1>the bitmasks needed to convert between a color and a mapped integer</font></li>
 <li><a href="surface.html#Surface.get_offset">Surface.get_offset</a> - <font size=-1>find the position of a child subsurface inside a parent</font></li>
Index: test/blit_test.py
===================================================================
--- test/blit_test.py	(Revision 1297)
+++ test/blit_test.py	(Arbeitskopie)
@@ -6,7 +6,6 @@
     def test_SRCALPHA( self ):
         """ SRCALPHA tests.
         """
-
         #blend(s, 0, d) = d
         s = pygame.Surface((1,1), SRCALPHA, 32)
         s.fill((255, 255,255, 0))
@@ -17,8 +16,6 @@
         s.blit(d, (0,0))
         self.assertEqual(s.get_at((0,0)), d.get_at((0,0)) )
 
-
-
         #blend(s, 255, d) = s
         s = pygame.Surface((1,1), SRCALPHA, 32)
         s.fill((123, 0, 0, 255))
@@ -29,13 +26,11 @@
         s.blit(d, (0,0))
         self.assertEqual(s.get_at((0,0)), s1.get_at((0,0)) )
 
-
         #TODO: these should be true too.
         #blend(0, sA, 0) = 0
         #blend(255, sA, 255) = 255
         #blend(s, sA, d) <= 255
 
-
     def test_BLEND( self ):
         """ BLEND_ tests.
         """
Index: test/surflock_test.py
===================================================================
--- test/surflock_test.py	(Revision 1297)
+++ test/surflock_test.py	(Arbeitskopie)
@@ -1,26 +1,26 @@
-import unittest
+import unittest, sys
 import pygame
 
 class SurfaceLockTest (unittest.TestCase):
 
-##     def test_lock (self):
-##         sf = pygame.Surface ((5, 5))
+    def test_lock (self):
+        sf = pygame.Surface ((5, 5))
 
-##         sf.lock ()
-##         self.assertEquals (sf.get_locked (), True)
-##         self.assertEquals (sf.get_locks (), (sf,))
+        sf.lock ()
+        self.assertEquals (sf.get_locked (), True)
+        self.assertEquals (sf.get_locks (), (sf,))
 
-##         sf.lock ()
-##         self.assertEquals (sf.get_locked (), True)
-##         self.assertEquals (sf.get_locks (), (sf,sf))
+        sf.lock ()
+        self.assertEquals (sf.get_locked (), True)
+        self.assertEquals (sf.get_locks (), (sf, sf))
 
-##         sf.unlock ()
-##         self.assertEquals (sf.get_locked (), True)
-##         self.assertEquals (sf.get_locks (), (sf,))
+        sf.unlock ()
+        self.assertEquals (sf.get_locked (), True)
+        self.assertEquals (sf.get_locks (), (sf,))
 
-##         sf.unlock ()
-##         self.assertEquals (sf.get_locked (), False)
-##         self.assertEquals (sf.get_locks (), ())
+        sf.unlock ()
+        self.assertEquals (sf.get_locked (), False)
+        self.assertEquals (sf.get_locks (), ())
 
     def test_subsurface_lock (self):
         sf = pygame.Surface ((5, 5))
@@ -57,51 +57,102 @@
 
         # Run a second unlock on the surface. This should ideally have
         # no effect as the subsurface is the locking reason!
-        # It has though and thus is wrong.
         sf.unlock ()
         self.assertRaises (pygame.error, sf2.blit, sf, (0, 0))
-
-        # Now the complete surface is unlocked and we can do any
-        # nastiness on it, while the subsurface still keeps a (now
-        # unnecessary) lock around. The same applies to other reference
-        # types such as buffers.
         self.assertRaises (pygame.error, sf2.blit, subsf, (0, 0))
+        subsf.unlock ()
+        
+        sf.lock ()
+        self.assertEquals (sf.get_locked (), True)
+        self.assertEquals (sf.get_locks (), (sf,))
+        self.assertEquals (subsf.get_locked (), False)
+        self.assertEquals (subsf.get_locks (), ())
 
-##         sf.lock ()
-##         self.assertEquals (sf.get_locked (), True)
-##         self.assertEquals (sf.get_locks (), (sf,))
-##         self.assertEquals (subsf.get_locked (), False)
-##         self.assertEquals (subsf.get_locks (), ())
+        subsf.lock ()
+        self.assertEquals (sf.get_locked (), True)
+        self.assertEquals (sf.get_locks (), (sf, subsf))
+        self.assertEquals (subsf.get_locked (), True)
+        self.assertEquals (subsf.get_locks (), (subsf,))
 
-##         subsf.lock ()
-##         self.assertEquals (sf.get_locked (), True)
-##         self.assertEquals (sf.get_locks (), (sf, subsf))
-##         self.assertEquals (subsf.get_locked (), True)
-##         self.assertEquals (subsf.get_locks (), (subsf,))
+        sf.unlock ()
+        self.assertEquals (sf.get_locked (), True)
+        self.assertEquals (sf.get_locks (), (subsf,))
+        self.assertEquals (subsf.get_locked (), True)
+        self.assertEquals (subsf.get_locks (), (subsf,))
 
-##         sf.unlock ()
-##         self.assertEquals (sf.get_locked (), True)
-##         self.assertEquals (sf.get_locks (), (subsf,))
-##         self.assertEquals (subsf.get_locked (), True)
-##         self.assertEquals (subsf.get_locks (), (subsf,))
+        subsf.unlock ()
+        self.assertEquals (sf.get_locked (), False)
+        self.assertEquals (sf.get_locks (), ())
+        self.assertEquals (subsf.get_locked (), False)
+        self.assertEquals (subsf.get_locks (), ())
 
-##         subsf.unlock ()
-##         self.assertEquals (sf.get_locked (), False)
-##         self.assertEquals (sf.get_locks (), ())
-##         self.assertEquals (subsf.get_locked (), False)
-##         self.assertEquals (subsf.get_locks (), ())
+        subsf.lock ()
+        self.assertEquals (sf.get_locked (), True)
+        self.assertEquals (sf.get_locks (), (subsf,))
+        self.assertEquals (subsf.get_locked (), True)
+        self.assertEquals (subsf.get_locks (), (subsf,))
 
-##         subsf.lock ()
-##         self.assertEquals (sf.get_locked (), True)
-##         self.assertEquals (sf.get_locks (), (subsf,))
-##         self.assertEquals (subsf.get_locked (), True)
-##         self.assertEquals (subsf.get_locks (), (subsf,))
+        subsf.lock ()
+        self.assertEquals (sf.get_locked (), True)
+        self.assertEquals (sf.get_locks (), (subsf, subsf))
+        self.assertEquals (subsf.get_locked (), True)
+        self.assertEquals (subsf.get_locks (), (subsf, subsf))
 
-##         subsf.lock ()
-##         self.assertEquals (sf.get_locked (), True)
-##         self.assertEquals (sf.get_locks (), (subsf, subsf))
-##         self.assertEquals (subsf.get_locked (), True)
-##         self.assertEquals (subsf.get_locks (), (subsf, subsf))
+    def test_pxarray_ref (self):
+        sf = pygame.Surface ((5, 5))
+        ar = pygame.PixelArray (sf)
+        ar2 = pygame.PixelArray (sf)
 
+        self.assertEquals (sf.get_locked (), True)
+        self.assertEquals (sf.get_locks (), (ar, ar2))
+
+        del ar
+        self.assertEquals (sf.get_locked (), True)
+        self.assertEquals (sf.get_locks (), (ar2,))
+
+        ar = ar2[:]
+        self.assertEquals (sf.get_locked (), True)
+        self.assertEquals (sf.get_locks (), (ar2,))
+
+        del ar
+        self.assertEquals (sf.get_locked (), True)
+        self.assertEquals (len (sf.get_locks ()), 1)
+
+    def test_buffer (self):
+        sf = pygame.Surface ((5, 5))
+        buf = sf.get_buffer ()
+
+        self.assertEquals (sf.get_locked (), True)
+        self.assertEquals (sf.get_locks (), (buf,))
+
+        sf.unlock ()
+        self.assertEquals (sf.get_locked (), True)
+        self.assertEquals (sf.get_locks (), (buf,))
+
+        del buf
+        self.assertEquals (sf.get_locked (), False)
+        self.assertEquals (sf.get_locks (), ())
+
+    def test_surfarray_ref (self):
+        sf = pygame.Surface ((5, 5))
+        for atype in pygame.surfarray.get_arraytypes ():
+            pygame.surfarray.use_arraytype (atype)
+            
+            ar = pygame.surfarray.pixels2d (sf)
+            self.assertEquals (sf.get_locked (), True)
+
+            # Numpy uses the Surface's buffer.
+            if atype == "numeric":
+                self.assertEquals (sf.get_locks (), (ar,))
+
+            sf.unlock ()
+            self.assertEquals (sf.get_locked (), True)
+
+            del ar
+            self.assertEquals (sf.get_locked (), False)
+            self.assertEquals (sf.get_locks (), ())
+
+        #print "test_surfarray_ref - end"
+
 if __name__ == '__main__':
     unittest.main()

Attachment: pgpghdCb99uAQ.pgp
Description: PGP signature