[Bf-committers] [Bf-blender-cvs] SVN commit: /data/svn/bf-blender [11005] branches/pyapi_devel/source/ blender: Solution for python referencing invalid ID pointers ( as discussed at the python meeting)

joe joeedh at gmail.com
Fri Jun 22 22:13:04 CEST 2007


BTW, I implemented something similar for the Draw module, for
pyconstraints.  It seemed to work great (solved all existing Draw
bugs) however I was still getting the occasional random crash
(possibly not related to that code) and what with time constraints I
havn't had time to clean it up and submit it.

The most useful thing is that the following code pattern:

st = ""
def draw():
    global st
    def exit_clicked(id, but):
        global st
        Draw.Exit()
    Draw.String("St:", 0, 10, 10, 70, 22, st, 32, "asaas", exit_clicked())

. . .Works.  At the moment, you have to put Draw.String into a global
variable, along with the tooltip, which is pretty clunky.

The Draw module actually has potential, I think, with these things fixed :)

Anyway, basically what I did is I stored references to button
pyobjects and their callbacks in a big list, stored per-draw-space.
Then, whenever a redraw event went out, the list would be cleared
(also when a script exited).  This way python button objects never got
prematurely deallocated, and also allowed storing the tooltip inside
the object (thus fixing our abuse of PyString_AsString).

I can provide a patch next week when I have access to my desktop
computer again, if anyone is interested in continuing the work.

Joe

On 6/22/07, Campbell Barton <cbarton at metavr.com> wrote:
> Revision: 11005
>           http://projects.blender.org/plugins/scmsvn/viewcvs.php?view=rev&root=bf-blender&revision=11005
> Author:   campbellbarton
> Date:     2007-06-22 15:08:03 +0200 (Fri, 22 Jun 2007)
>
> Log Message:
> -----------
> Solution for python referencing invalid ID pointers (as discussed at the python meeting)
> http://wiki.blender.org/index.php/Rewriting_the_2.4x_BPython_API#Managing_Pointers
>
> Only work on text and scenes at the moment
>
> Modified Paths:
> --------------
>     branches/pyapi_devel/source/blender/blenkernel/intern/library.c
>     branches/pyapi_devel/source/blender/python/BPY_extern.h
>     branches/pyapi_devel/source/blender/python/BPY_interface.c
>     branches/pyapi_devel/source/blender/python/api2_2x/Scene.c
>     branches/pyapi_devel/source/blender/python/api2_2x/Text.c
>     branches/pyapi_devel/source/blender/python/api2_2x/layer_set.c
>     branches/pyapi_devel/source/blender/python/api2_2x/layer_set.h
>
> Modified: branches/pyapi_devel/source/blender/blenkernel/intern/library.c
> ===================================================================
> --- branches/pyapi_devel/source/blender/blenkernel/intern/library.c     2007-06-22 11:55:00 UTC (rev 11004)
> +++ branches/pyapi_devel/source/blender/blenkernel/intern/library.c     2007-06-22 13:08:03 UTC (rev 11005)
> @@ -114,6 +114,9 @@
>
>  #include "BPI_script.h"
>
> +/* for maintaining the python id hash when removign data */
> +#include "BPY_extern.h"
> +
>  #define MAX_IDPUP              60      /* was 24 */
>
>  /* ************* general ************************ */
> @@ -423,6 +426,9 @@
>  {
>         ID *id= idv;
>
> +       /* invalidate the data if its in the python pool */
> +       BPY_idhash_invalidate(id);
> +
>         switch( GS(id->name) ) {        /* GetShort from util.h */
>                 case ID_SCE:
>                         free_scene((Scene *)id);
>
> Modified: branches/pyapi_devel/source/blender/python/BPY_extern.h
> ===================================================================
> --- branches/pyapi_devel/source/blender/python/BPY_extern.h     2007-06-22 11:55:00 UTC (rev 11004)
> +++ branches/pyapi_devel/source/blender/python/BPY_extern.h     2007-06-22 13:08:03 UTC (rev 11005)
> @@ -96,6 +96,12 @@
>         void BPY_clear_script( struct Script *script );
>         void BPY_free_finished_script( struct Script *script );
>
> +/* tell python were removing this ID so python can NULL the pointers */
> +       void BPY_idhash_add(void * value);
> +       void *BPY_idhash_get(ID *id);
> +       void BPY_idhash_remove(ID *id);
> +       void BPY_idhash_invalidate(ID *id);
> +
>  /* void BPY_Err_Handle(struct Text *text); */
>  /* void BPY_clear_bad_scriptlink(struct ID *id, struct Text *byebye); */
>  /* void BPY_clear_bad_scriptlist(struct ListBase *, struct Text *byebye); */
>
> Modified: branches/pyapi_devel/source/blender/python/BPY_interface.c
> ===================================================================
> --- branches/pyapi_devel/source/blender/python/BPY_interface.c  2007-06-22 11:55:00 UTC (rev 11004)
> +++ branches/pyapi_devel/source/blender/python/BPY_interface.c  2007-06-22 13:08:03 UTC (rev 11005)
> @@ -36,6 +36,7 @@
>  #include "compile.h"           /* for the PyCodeObject */
>  #include "eval.h"              /* for PyEval_EvalCode */
>  #include "BLI_blenlib.h"       /* for BLI_last_slash() */
> +#include "BLI_ghash.h" /* for tracking removed data */
>  #include "BIF_interface.h"     /* for pupmenu() */
>  #include "BIF_space.h"
>  #include "BIF_screen.h"
> @@ -68,6 +69,10 @@
>  #include "api2_2x/Registry.h"
>  #include "api2_2x/bpy.h" /* for the new "bpy" module */
>
> +#include "api2_2x/gen_library.h"
> +#include "blendef.h"
> +#include "BKE_utildefines.h"
> +
>  /* for scriptlinks */
>  #include "DNA_lamp_types.h"
>  #include "DNA_camera_types.h"
> @@ -75,6 +80,8 @@
>  #include "DNA_scene_types.h"
>  #include "DNA_material_types.h"
>
> +#include "DNA_ID.h"
> +
>  /* bpy_registryDict is declared in api2_2x/Registry.h and defined
>   * in api2_2x/Registry.c
>   * This Python dictionary will be used to store data that scripts
> @@ -154,7 +161,79 @@
>  void BPY_Err_Handle( char *script_name );
>  PyObject *traceback_getFilename( PyObject * tb );
>
> +
>  /****************************************************************************
> +* Description: These functions deal with maintaining a python hash of ID
> +* types that can be removed. This is important because when blender data is
> +* removed, python needs to invalidate the BPy data by setting its ID pointer
> +* to NULL, after this, using this data will raise an error.
> +****************************************************************************/
> +
> +const GHash *bpy_idhash_text;
> +const GHash *bpy_idhash_scene;
> +const GHash *bpy_idhash_object;
> +const GHash *bpy_idhash_group;
> +
> +static GHash * idhash__internal(ID *id)
> +{
> +       switch (GS(id->name)) {
> +       case ID_TXT:
> +               return bpy_idhash_text;
> +       case ID_SCE:
> +               return bpy_idhash_scene;
> +       case ID_OB:
> +               return bpy_idhash_object;
> +       case ID_GR:
> +               return bpy_idhash_group;
> +       }
> +       return NULL; /* WARNING - this will crash BLI_ghash_lookup */
> +}
> +
> +/* Use so duplicate PyObjects are never created */
> +void * BPY_idhash_get(ID *id)
> +{
> +       printf("getting hash\n");
> +       return BLI_ghash_lookup(idhash__internal(id), id);
> +}
> +/* Use when python decref's the data
> + * no need to worry about a loose pointer because python
> + * isnt referencing any more */
> +void BPY_idhash_remove(ID *id)
> +{
> +       GHash *hash = idhash__internal(id);
> +       if (!hash) /* TODO - Dont allow invalid hashes at all*/
> +               return;
> +
> +       BLI_ghash_remove(hash, id, NULL, NULL);
> +}
> +
> +static void genlib_invalidate(void * genlib)
> +{
> +       ((BPy_GenericLib *)genlib)->id = NULL;
> +}
> +
> +/* Use when removing the data data */
> +void BPY_idhash_invalidate(ID *id)
> +{
> +       GHash *hash = idhash__internal(id);
> +       if (!hash) /* TODO - Dont allow invalid hashes */
> +               return;
> +
> +       BLI_ghash_remove(hash, id, NULL, genlib_invalidate);
> +
> +}
> +
> +/* Use when creating a new pyobject */
> +void BPY_idhash_add(void *value)
> +{
> +       ID *id = ((BPy_GenericLib *)value)->id;
> +       printf("adding hash\n");
> +       BLI_ghash_insert(idhash__internal(id), (void *)id, (void *)value);
> +}
> +/* END OF BPY ID HASH */
> +
> +
> +/****************************************************************************
>  * Description: This function will start the interpreter and load all modules
>  * as well as search for a python installation.
>  ****************************************************************************/
> @@ -214,7 +293,14 @@
>
>         //Look for a python installation
>         init_syspath( first_time ); /* not first_time: some msgs are suppressed */
> -
> +
> +       // init hash
> +       printf("initializing HASH\n");
> +       bpy_idhash_text = BLI_ghash_new(BLI_ghashutil_ptrhash, BLI_ghashutil_ptrcmp);
> +       bpy_idhash_scene = BLI_ghash_new(BLI_ghashutil_ptrhash, BLI_ghashutil_ptrcmp);
> +       bpy_idhash_object = BLI_ghash_new(BLI_ghashutil_ptrhash, BLI_ghashutil_ptrcmp);
> +       bpy_idhash_group = BLI_ghash_new(BLI_ghashutil_ptrhash, BLI_ghashutil_ptrcmp);
> +
>         return;
>  }
>
> @@ -249,6 +335,12 @@
>         /* a script might've opened a .blend file but didn't close it, so: */
>         EXPP_Library_Close(  );
>
> +       // free id hashes
> +       BLI_ghash_free(bpy_idhash_text, NULL, NULL);
> +       BLI_ghash_free(bpy_idhash_scene, NULL, NULL);
> +       BLI_ghash_free(bpy_idhash_group, NULL, NULL);
> +       BLI_ghash_free(bpy_idhash_object, NULL, NULL);
> +
>         return;
>  }
>
>
> Modified: branches/pyapi_devel/source/blender/python/api2_2x/Scene.c
> ===================================================================
> --- branches/pyapi_devel/source/blender/python/api2_2x/Scene.c  2007-06-22 11:55:00 UTC (rev 11004)
> +++ branches/pyapi_devel/source/blender/python/api2_2x/Scene.c  2007-06-22 13:08:03 UTC (rev 11005)
> @@ -283,6 +283,20 @@
>  };
>
>
> +/*
> + * Scene dealloc - free from memory and free from text pool
> + */
> +static void Scene_dealloc( BPy_Scene * self )
> +{
> +       ID *id = (ID *)(self->scene);
> +       if (id) {
> +               BPY_idhash_remove(id);
> +       }
> +
> +       PyObject_DEL( self );
> +}
> +
> +
>  /*-----------------------BPy_Scene method def------------------------------*/
>  PyTypeObject Scene_Type = {
>         PyObject_HEAD_INIT( NULL )
> @@ -291,7 +305,7 @@
>         sizeof( BPy_Scene ),    /* tp_basicsize */
>         0,                      /* tp_itemsize */
>         /* methods */
> -       NULL,                                           /* tp_dealloc */
> +       ( destructor ) Scene_dealloc,   /* tp_dealloc */
>         NULL,                       /* printfunc tp_print; */
>         NULL,                       /* getattrfunc tp_getattr; */
>         NULL,                       /* setattrfunc tp_setattr; */
> @@ -395,7 +409,7 @@
>  static PyObject *Scene_repr( BPy_Scene * self )
>  {
>         if( !(self->scene) )
> -               return PyString_FromString( "[Scene - Removed]");
> +               return PyString_FromString( "[Scene <deleted>]");
>         else
>                 return PyString_FromFormat( "[Scene \"%s\"]",
>                                         self->scene->id.name + 2 );
> @@ -406,6 +420,11 @@
>  {
>         BPy_Scene *pyscene;
>
> +       /* REUSE EXISTING DATA FROM HASH */
> +       pyscene = BPY_idhash_get((ID *)scene);
> +       if (pyscene)
> +               return EXPP_incr_ret((PyObject *)pyscene);
> +
>         pyscene = ( BPy_Scene * ) PyObject_NEW( BPy_Scene, &Scene_Type );
>
>         if( !pyscene )
> @@ -414,6 +433,8 @@
>
>         pyscene->scene = scene;
>
> +       BPY_idhash_add((void *)pyscene);
> +
>         return ( PyObject * ) pyscene;
>  }
>
>
> Modified: branches/pyapi_devel/source/blender/python/api2_2x/Text.c
> ===================================================================
> --- branches/pyapi_devel/source/blender/python/api2_2x/Text.c   2007-06-22 11:55:00 UTC (rev 11004)
> +++ branches/pyapi_devel/source/blender/python/api2_2x/Text.c   2007-06-22 13:08:03 UTC (rev 11005)
> @@ -119,7 +119,12 @@
>  PyObject *Text_CreatePyObject( Text * txt )
>  {
>         BPy_Text *pytxt;
> -
> +
> +       /* REUSE EXISTING DATA FROM HASH */
> +       pytxt = BPY_idhash_get((ID *)txt);
> +       if (pytxt)
> +               return EXPP_incr_ret((PyObject *)pytxt);
> +
>         pytxt = ( BPy_Text * ) PyObject_NEW( BPy_Text, &Text_Type );
>
>         if( !pytxt )
> @@ -128,6 +133,8 @@
>
>         pytxt->text = txt;
>
> +       BPY_idhash_add((void *)pytxt);
> +
>         return ( PyObject * ) pytxt;
>  }
>
> @@ -298,7 +305,6 @@
>  /*****************************************************************************/
>  static PyGetSetDef BPy_Text_getseters[] = {
>         GENERIC_LIB_GETSETATTR,
> -       GENERIC_LIB_GETSETATTR_MATERIAL,
>         {"filename", (getter)Text_getFilename, (setter)NULL,
>          "text filename", NULL},
>         {"mode", (getter)Text_getMode, (setter)NULL,
> @@ -308,6 +314,19 @@
>         {NULL,NULL,NULL,NULL,NULL}  /* Sentinel */
>  };
>
> +/*
> + * Text dealloc - free from memory and free from text pool
> + */
> +static void Text_dealloc( BPy_Text * self )
> +{
> +       ID *id = (ID *)(self->text);
> +       if (id) {
> +               BPY_idhash_remove(id);
> +       }
> +
> +       PyObject_DEL( self );
> +}
> +
>  /*****************************************************************************/
>  /* Python Text_Type structure definition:                                    */
>  /*****************************************************************************/
> @@ -318,7 +337,7 @@
>         sizeof( BPy_Text ),     /* tp_basicsize */
>         0,                      /* tp_itemsize */
>         /* methods */
> -       NULL,   /* tp_dealloc */
> +       ( destructor ) Text_dealloc,    /* tp_dealloc */
>         NULL,                   /* tp_print */
>         NULL,   /* tp_getattr */
>         NULL,   /* tp_setattr */
>
> Modified: branches/pyapi_devel/source/blender/python/api2_2x/layer_set.c
> ===================================================================
> --- branches/pyapi_devel/source/blender/python/api2_2x/layer_set.c      2007-06-22 11:55:00 UTC (rev 11004)
> +++ branches/pyapi_devel/source/blender/python/api2_2x/layer_set.c      2007-06-22 13:08:03 UTC (rev 11005)
> @@ -210,13 +210,13 @@
>  static int layer_from_iter__internal(PyObject *seq)
>  {
>         PyObject *item, *iter;
> -       int layer = 0, bit;
> +       int layer = 0, bit;
>
> -       if (!PyIter_Check(seq))
> +       iter = PyObject_GetIter(seq);
> +
> +       if (!iter)
>                 return -1;
>
> -       iter = PyObject_GetIter(seq);
> -
>         item = PyIter_Next(iter);
>
>         while (item) {
> @@ -240,10 +240,27 @@
>         return layer; /* zero is ok here */
>  }
>
>
> @@ Diff output truncated at 10240 characters. @@
>
> _______________________________________________
> Bf-blender-cvs mailing list
> Bf-blender-cvs at blender.org
> http://lists.blender.org/mailman/listinfo/bf-blender-cvs
>


More information about the Bf-committers mailing list