Change Details

commit 591488c3f0de3b414df0470b8b78b24de26df586 Author: Campbell Barton <ideasman42@gmail.com> Date: Wed Oct 13 22:56:40 2021 +1100 Fix T92136: Leak accessing evaluated depsgraph data from Python Copy-on-write data blocks could be referenced from python but were not properly managing python reference counting. This would leak memory for any evaluated data-blocks accessed by Python. This patch removes the reference on free, backing up the reference in deg_expand_copy_on_write_datablock since the ID is only being updated so the reference can be kept active. diff --git a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc index b4a91944b65..c35916a0674 100644 --- a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc +++ b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc @@ -817,6 +817,13 @@ ID *deg_expand_copy_on_write_datablock(const Depsgraph *depsgraph, const IDNode return id_cow; } + /* Avoid removing & re-creating the reference if it exists. + * Without this, the call to deg_free_copy_on_write_datablock may clear the reference + * unnecessarily, since the data is being updated and the ID is not being freed, + * restore this reference once the data has been restored in-place. */ + void *py_instance = id_cow->py_instance; + id_cow->py_instance = nullptr; + DEG_COW_PRINT( "Expanding datablock for %s: id_orig=%p id_cow=%p\n", id_orig->name, id_orig, id_cow); @@ -884,6 +891,7 @@ ID *deg_expand_copy_on_write_datablock(const Depsgraph *depsgraph, const IDNode * from above. */ update_id_after_copy(depsgraph, id_node, id_orig, id_cow); id_cow->recalc = id_cow_recalc; + id_cow->py_instance = py_instance; return id_cow; } @@ -1015,6 +1023,7 @@ void deg_free_copy_on_write_datablock(ID *id_cow) break; } discard_edit_mode_pointers(id_cow); + BKE_libblock_free_data_py(id_cow); BKE_libblock_free_datablock(id_cow, 0); BKE_libblock_free_data(id_cow, false); /* Signal datablock as not being expanded. */ diff --git a/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup.cc b/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup.cc index 7893e8c64c1..8bf64af7d5d 100644 --- a/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup.cc +++ b/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup.cc @@ -52,7 +52,9 @@ void RuntimeBackup::init_from_id(ID *id) } have_backup = true; + /* Clear, so freeing the expanded data doesn't touch this Python reference. */ id_data.py_instance = id->py_instance; + id->py_instance = nullptr; animation_backup.init_from_id(id);

commit a722e17b51edf363f7d7b11b3868c5db7e719244 Author: Campbell Barton <ideasman42@gmail.com> Date: Wed Oct 13 22:56:40 2021 +1100 Fix T92136: Leak accessing evaluated depsgraph data from Python Copy-on-write data blocks could be referenced from python but were not properly managing python reference counting. This would leak memory for any evaluated data-blocks accessed by Python. This patch removes the reference on free, backing up the reference in deg_expand_copy_on_write_datablock since the ID is only being updated so the reference can be kept active. diff --git a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc index b4a91944b65..68a72638c7d 100644 --- a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc +++ b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc @@ -822,6 +822,7 @@ ID *deg_expand_copy_on_write_datablock(const Depsgraph *depsgraph, const IDNode /* Sanity checks. */ BLI_assert(check_datablock_expanded(id_cow) == false); + BLI_assert(id_cow->py_instance == nullptr); /* Copy data from original ID to a copied version. */ /* TODO(sergey): Avoid doing full ID copy somehow, make Mesh to reference @@ -1015,6 +1016,7 @@ void deg_free_copy_on_write_datablock(ID *id_cow) break; } discard_edit_mode_pointers(id_cow); + BKE_libblock_free_data_py(id_cow); BKE_libblock_free_datablock(id_cow, 0); BKE_libblock_free_data(id_cow, false); /* Signal datablock as not being expanded. */ diff --git a/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup.cc b/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup.cc index 7893e8c64c1..8bf64af7d5d 100644 --- a/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup.cc +++ b/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup.cc @@ -52,7 +52,9 @@ void RuntimeBackup::init_from_id(ID *id) } have_backup = true; + /* Clear, so freeing the expanded data doesn't touch this Python reference. */ id_data.py_instance = id->py_instance; + id->py_instance = nullptr; animation_backup.init_from_id(id);

commit 591488c3f0de3b414df0470b8b78b24de26df586a722e17b51edf363f7d7b11b3868c5db7e719244 Author: Campbell Barton <ideasman42@gmail.com> Date: Wed Oct 13 22:56:40 2021 +1100 Fix T92136: Leak accessing evaluated depsgraph data from Python Copy-on-write data blocks could be referenced from python but were not properly managing python reference counting. This would leak memory for any evaluated data-blocks accessed by Python. This patch removes the reference on free, backing up the reference in deg_expand_copy_on_write_datablock since the ID is only being updated so the reference can be kept active. diff --git a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc index b4a91944b65..c35916a067468a72638c7d 100644 --- a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc +++ b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc @@ -81722,6 +817,1322,7 @@ ID *deg_expand_copy_on_write_datablock(const Depsgraph *depsgraph, const IDNode return id_cow; } + /* Avoid removing & re-creating the reference if it exists. + * Without this, the call to deg_free_copy_on_write_datablock may clear the reference + * unnecessarily, since the data is being updated and the ID is not being freed, + * restore this reference once the data has been restored in-place /* Sanity checks. */ + void *py_instance = id_cow->py_instance; + id_cow->py_instance = nullptr; + DEG_COW_PRINT(BLI_assert(check_datablock_expanded(id_cow) == false); "Expanding datablock for %s: id_orig=%p + BLI_assert(id_cow=%p\n", id_orig->name, id_orig, id_cow);->py_instance == nullptr); @@ -884,6 +891,7 @@ ID *deg_expand_copy_on_write_datablock(const Depsgraph *depsgraph, const IDNode * from above. */ update_id_after_copy(depsgraph, id_node, id_orig, id_cow); id_cow->recalc = id_cow_recalc; + id_cow->py_instance = py_instance;/* Copy data from original ID to a copied version. */ return id_cow;/* TODO(sergey): Avoid doing full ID copy somehow, make Mesh to reference } @@ -1015,6 +102316,7 @@ void deg_free_copy_on_write_datablock(ID *id_cow) break; } discard_edit_mode_pointers(id_cow); + BKE_libblock_free_data_py(id_cow); BKE_libblock_free_datablock(id_cow, 0); BKE_libblock_free_data(id_cow, false); /* Signal datablock as not being expanded. */ diff --git a/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup.cc b/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup.cc index 7893e8c64c1..8bf64af7d5d 100644 --- a/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup.cc +++ b/source/blender/depsgraph/intern/eval/deg_eval_runtime_backup.cc @@ -52,7 +52,9 @@ void RuntimeBackup::init_from_id(ID *id) } have_backup = true; + /* Clear, so freeing the expanded data doesn't touch this Python reference. */ id_data.py_instance = id->py_instance; + id->py_instance = nullptr; animation_backup.init_from_id(id);