Fix T37709: Memory corruption when freeing custom bone shape objects
authorSergey Sharybin <sergey.vfx@gmail.com>
Wed, 25 Dec 2013 10:43:26 +0000 (16:43 +0600)
committerSergey Sharybin <sergey.vfx@gmail.com>
Wed, 25 Dec 2013 10:43:26 +0000 (16:43 +0600)
Summary:
Issue was caused by access to pchan->custom object from channel free
function when freeing all objects from main. Order of objects free
is not defined and such an access might easily end up with access
to freed memory.

We don't need to do user counter stuff when freeing main, so added
an _ex functions with do_id_user flag which is used when freeing main.

We had the same issue with other datablocks, so now it should be
easier to support relevant user counter.

This issue was caused by the fix for T36391, so perhaps that's indeed
high time to do real user counter.

Reviewers: brecht, campbellbarton

Reviewed By: campbellbarton

Maniphest Tasks: T37709

Differential Revision: https://developer.blender.org/D137

source/blender/blenkernel/BKE_action.h
source/blender/blenkernel/BKE_library.h
source/blender/blenkernel/BKE_object.h
source/blender/blenkernel/intern/action.c
source/blender/blenkernel/intern/library.c
source/blender/blenkernel/intern/object.c

index 3ac5c8c9a76979b02089da9a2da6cb7bf3ad76c4..9068be970b54c943a89fe33af528a747db14e6aa 100644 (file)
@@ -134,13 +134,16 @@ void action_groups_clear_tempflags(struct bAction *act);
 /* Pose API ----------------- */       
 
 void                 BKE_pose_channel_free(struct bPoseChannel *pchan);
+void                 BKE_pose_channel_free_ex(struct bPoseChannel *pchan, bool do_id_user);
 
 void                 BKE_pose_channels_free(struct bPose *pose);
+void                 BKE_pose_channels_free_ex(struct bPose *pose, bool do_id_user);
 
 void                 BKE_pose_channels_hash_make(struct bPose *pose);
 void                 BKE_pose_channels_hash_free(struct bPose *pose);
 
 void                 BKE_pose_free(struct bPose *pose);
+void                 BKE_pose_free_ex(struct bPose *pose, bool do_id_user);
 void                 BKE_pose_copy_data(struct bPose **dst, struct bPose *src, const bool copy_constraints);
 void                 BKE_pose_channel_copy_data(struct bPoseChannel *pchan, const struct bPoseChannel *pchan_from);
 struct bPoseChannel *BKE_pose_channel_find_name(const struct bPose *pose, const char *name);
index b8da5002d841cd6a3228e2e167636742d7194beb..f97bb911a1293d7cc4282d054f49c76a1e13f636 100644 (file)
@@ -74,6 +74,7 @@ struct ListBase *which_libbase(struct Main *mainlib, short type);
 int set_listbasepointers(struct Main *main, struct ListBase **lb);
 
 void BKE_libblock_free(struct ListBase *lb, void *idv);
+void BKE_libblock_free_ex(struct ListBase *lb, void *idv, bool do_id_user);
 void BKE_libblock_free_us(struct ListBase *lb, void *idv);
 void BKE_libblock_free_data(struct ID *id);
 void free_main(struct Main *mainvar);
index e6e6d621ef37ac73f0b1aa7b6d7aad11ea8661d6..94995404f2be94f21176a6b97d0daf1f0db9071c 100644 (file)
@@ -65,6 +65,7 @@ void BKE_object_free_curve_cache(struct Object *ob);
 void BKE_object_update_base_layer(struct Scene *scene, struct Object *ob);
 
 void BKE_object_free(struct Object *ob);
+void BKE_object_free_ex(struct Object *ob, bool do_id_user);
 void BKE_object_free_derived_caches(struct Object *ob);
 
 void BKE_object_modifier_hook_reset(struct Object *ob, struct HookModifierData *hmd);
index c91fae2adbcfb0f345d3722c84b578239225d860..4991f1ce225d7088b988cbf4a27566a4ece9dc20 100644 (file)
@@ -714,10 +714,12 @@ void BKE_pose_channels_hash_free(bPose *pose)
  * Deallocates a pose channel.
  * Does not free the pose channel itself.
  */
-void BKE_pose_channel_free(bPoseChannel *pchan)
+void BKE_pose_channel_free_ex(bPoseChannel *pchan, bool do_id_user)
 {
        if (pchan->custom) {
-               id_us_min(&pchan->custom->id);
+               if (do_id_user) {
+                       id_us_min(&pchan->custom->id);
+               }
                pchan->custom = NULL;
        }
 
@@ -734,17 +736,22 @@ void BKE_pose_channel_free(bPoseChannel *pchan)
        }
 }
 
+void BKE_pose_channel_free(bPoseChannel *pchan)
+{
+       BKE_pose_channel_free_ex(pchan, true);
+}
+
 /**
  * Removes and deallocates all channels from a pose.
  * Does not free the pose itself.
  */
-void BKE_pose_channels_free(bPose *pose) 
+void BKE_pose_channels_free_ex(bPose *pose, bool do_id_user)
 {
        bPoseChannel *pchan;
        
        if (pose->chanbase.first) {
                for (pchan = pose->chanbase.first; pchan; pchan = pchan->next)
-                       BKE_pose_channel_free(pchan);
+                       BKE_pose_channel_free_ex(pchan, do_id_user);
                
                BLI_freelistN(&pose->chanbase);
        }
@@ -752,14 +759,19 @@ void BKE_pose_channels_free(bPose *pose)
        BKE_pose_channels_hash_free(pose);
 }
 
+void BKE_pose_channels_free(bPose *pose)
+{
+       BKE_pose_channels_free_ex(pose, true);
+}
+
 /**
  * Removes and deallocates all data from a pose, and also frees the pose.
  */
-void BKE_pose_free(bPose *pose)
+void BKE_pose_free_ex(bPose *pose, bool do_id_user)
 {
        if (pose) {
                /* free pose-channels */
-               BKE_pose_channels_free(pose);
+               BKE_pose_channels_free_ex(pose, do_id_user);
                
                /* free pose-groups */
                if (pose->agroups.first)
@@ -777,6 +789,11 @@ void BKE_pose_free(bPose *pose)
        }
 }
 
+void BKE_pose_free(bPose *pose)
+{
+       BKE_pose_free_ex(pose, true);
+}
+
 static void copy_pose_channel_data(bPoseChannel *pchan, const bPoseChannel *chan)
 {
        bConstraint *pcon, *con;
index 9b8d34e651dbf694ec3d20ad2e92ea072f5329c0..95f1cce38e3d2b0aed282b1f5a3118fc3c3df6df 100644 (file)
@@ -867,7 +867,7 @@ void BKE_libblock_free_data(ID *id)
 }
 
 /* used in headerbuttons.c image.c mesh.c screen.c sound.c and library.c */
-void BKE_libblock_free(ListBase *lb, void *idv)
+void BKE_libblock_free_ex(ListBase *lb, void *idv, bool do_id_user)
 {
        Main *bmain = G.main;  /* should eventually be an arg */
        ID *id = idv;
@@ -884,7 +884,7 @@ void BKE_libblock_free(ListBase *lb, void *idv)
                        BKE_library_free((Library *)id);
                        break;
                case ID_OB:
-                       BKE_object_free((Object *)id);
+                       BKE_object_free_ex((Object *)id, do_id_user);
                        break;
                case ID_ME:
                        BKE_mesh_free((Mesh *)id, 1);
@@ -987,6 +987,11 @@ void BKE_libblock_free(ListBase *lb, void *idv)
        MEM_freeN(id);
 }
 
+void BKE_libblock_free(ListBase *lb, void *idv)
+{
+       BKE_libblock_free_ex(lb, idv, true);
+}
+
 void BKE_libblock_free_us(ListBase *lb, void *idv)      /* test users */
 {
        ID *id = idv;
@@ -1018,44 +1023,44 @@ void free_main(Main *mainvar)
                
                while ( (id = lb->first) ) {
 #if 1
-                       BKE_libblock_free(lb, id);
+                       BKE_libblock_free_ex(lb, id, false);
 #else
                        /* errors freeing ID's can be hard to track down,
                         * enable this so valgrind will give the line number in its error log */
                        switch (a) {
-                               case   0: BKE_libblock_free(lb, id); break;
-                               case   1: BKE_libblock_free(lb, id); break;
-                               case   2: BKE_libblock_free(lb, id); break;
-                               case   3: BKE_libblock_free(lb, id); break;
-                               case   4: BKE_libblock_free(lb, id); break;
-                               case   5: BKE_libblock_free(lb, id); break;
-                               case   6: BKE_libblock_free(lb, id); break;
-                               case   7: BKE_libblock_free(lb, id); break;
-                               case   8: BKE_libblock_free(lb, id); break;
-                               case   9: BKE_libblock_free(lb, id); break;
-                               case  10: BKE_libblock_free(lb, id); break;
-                               case  11: BKE_libblock_free(lb, id); break;
-                               case  12: BKE_libblock_free(lb, id); break;
-                               case  13: BKE_libblock_free(lb, id); break;
-                               case  14: BKE_libblock_free(lb, id); break;
-                               case  15: BKE_libblock_free(lb, id); break;
-                               case  16: BKE_libblock_free(lb, id); break;
-                               case  17: BKE_libblock_free(lb, id); break;
-                               case  18: BKE_libblock_free(lb, id); break;
-                               case  19: BKE_libblock_free(lb, id); break;
-                               case  20: BKE_libblock_free(lb, id); break;
-                               case  21: BKE_libblock_free(lb, id); break;
-                               case  22: BKE_libblock_free(lb, id); break;
-                               case  23: BKE_libblock_free(lb, id); break;
-                               case  24: BKE_libblock_free(lb, id); break;
-                               case  25: BKE_libblock_free(lb, id); break;
-                               case  26: BKE_libblock_free(lb, id); break;
-                               case  27: BKE_libblock_free(lb, id); break;
-                               case  28: BKE_libblock_free(lb, id); break;
-                               case  29: BKE_libblock_free(lb, id); break;
-                               case  30: BKE_libblock_free(lb, id); break;
-                               case  31: BKE_libblock_free(lb, id); break;
-                               case  32: BKE_libblock_free(lb, id); break;
+                               case   0: BKE_libblock_free_ex(lb, id, false); break;
+                               case   1: BKE_libblock_free_ex(lb, id, false); break;
+                               case   2: BKE_libblock_free_ex(lb, id, false); break;
+                               case   3: BKE_libblock_free_ex(lb, id, false); break;
+                               case   4: BKE_libblock_free_ex(lb, id, false); break;
+                               case   5: BKE_libblock_free_ex(lb, id, false); break;
+                               case   6: BKE_libblock_free_ex(lb, id, false); break;
+                               case   7: BKE_libblock_free_ex(lb, id, false); break;
+                               case   8: BKE_libblock_free_ex(lb, id, false); break;
+                               case   9: BKE_libblock_free_ex(lb, id, false); break;
+                               case  10: BKE_libblock_free_ex(lb, id, false); break;
+                               case  11: BKE_libblock_free_ex(lb, id, false); break;
+                               case  12: BKE_libblock_free_ex(lb, id, false); break;
+                               case  13: BKE_libblock_free_ex(lb, id, false); break;
+                               case  14: BKE_libblock_free_ex(lb, id, false); break;
+                               case  15: BKE_libblock_free_ex(lb, id, false); break;
+                               case  16: BKE_libblock_free_ex(lb, id, false); break;
+                               case  17: BKE_libblock_free_ex(lb, id, false); break;
+                               case  18: BKE_libblock_free_ex(lb, id, false); break;
+                               case  19: BKE_libblock_free_ex(lb, id, false); break;
+                               case  20: BKE_libblock_free_ex(lb, id, false); break;
+                               case  21: BKE_libblock_free_ex(lb, id, false); break;
+                               case  22: BKE_libblock_free_ex(lb, id, false); break;
+                               case  23: BKE_libblock_free_ex(lb, id, false); break;
+                               case  24: BKE_libblock_free_ex(lb, id, false); break;
+                               case  25: BKE_libblock_free_ex(lb, id, false); break;
+                               case  26: BKE_libblock_free_ex(lb, id, false); break;
+                               case  27: BKE_libblock_free_ex(lb, id, false); break;
+                               case  28: BKE_libblock_free_ex(lb, id, false); break;
+                               case  29: BKE_libblock_free_ex(lb, id, false); break;
+                               case  30: BKE_libblock_free_ex(lb, id, false); break;
+                               case  31: BKE_libblock_free_ex(lb, id, false); break;
+                               case  32: BKE_libblock_free_ex(lb, id, false); break;
                                default:
                                        BLI_assert(0);
                                        break;
index 8657e3b0ebfac0313e29aa4c1634a9bcf20dec92..e480f1a6bf7d30f6d77d356ca04bfb6439fdf4f1 100644 (file)
@@ -321,7 +321,7 @@ void BKE_object_free_derived_caches(Object *ob)
 }
 
 /* do not free object itself */
-void BKE_object_free(Object *ob)
+void BKE_object_free_ex(Object *ob, bool do_id_user)
 {
        int a;
        
@@ -364,7 +364,7 @@ void BKE_object_free(Object *ob)
        if (ob->defbase.first)
                BLI_freelistN(&ob->defbase);
        if (ob->pose)
-               BKE_pose_free(ob->pose);
+               BKE_pose_free_ex(ob->pose, do_id_user);
        if (ob->mpath)
                animviz_free_motionpath(ob->mpath);
        BKE_bproperty_free_list(&ob->prop);
@@ -399,6 +399,11 @@ void BKE_object_free(Object *ob)
        }
 }
 
+void BKE_object_free(Object *ob)
+{
+       BKE_object_free_ex(ob, true);
+}
+
 static void unlink_object__unlinkModifierLinks(void *userData, Object *ob, Object **obpoin)
 {
        Object *unlinkOb = userData;