Fix (unreported) potential invalid memory access in BKE_main_collection_sync_remap.

After remapping there is no guarantee that collections' parent pointers
are still valid, so using these to free the object cache of all the
collections' ancestors is potentially accessing invalid memory.

Further more, it is noticiably inefficient, as a same collection may be
processed many times.

So instead, introduce (and use in resync case) a new
`BKE_main_collections_object_cache_free`, which iterates over all
collections of given Main and only free their own cache.
This commit is contained in:
Bastien Montagne
2023-03-21 18:38:04 +01:00
parent e858be84fa
commit 5bb4d733a6
3 changed files with 23 additions and 5 deletions

View File

@@ -217,7 +217,11 @@ bool BKE_collection_object_cyclic_check(struct Main *bmain,
struct ListBase BKE_collection_object_cache_get(struct Collection *collection);
ListBase BKE_collection_object_cache_instanced_get(struct Collection *collection);
/** Free the object cache of given `collection` and all of its ancestors (recursively). */
void BKE_collection_object_cache_free(struct Collection *collection);
/** Free the object cache of all collections in given `bmain`, including master collections of
* scenes. */
void BKE_main_collections_object_cache_free(const struct Main *bmain);
struct Base *BKE_collection_or_layer_objects(const struct Scene *scene,
struct ViewLayer *view_layer,

View File

@@ -82,6 +82,8 @@ static CollectionParent *collection_find_parent(Collection *child, Collection *c
static bool collection_find_child_recursive(const Collection *parent,
const Collection *collection);
static void collection_object_cache_free(Collection *collection);
static void collection_gobject_hash_ensure(Collection *collection);
static void collection_gobject_hash_update_object(Collection *collection,
Object *ob_old,
@@ -160,7 +162,7 @@ static void collection_free_data(ID *id)
BLI_freelistN(&collection->children);
BLI_freelistN(&collection->runtime.parents);
BKE_collection_object_cache_free(collection);
collection_object_cache_free(collection);
}
static void collection_foreach_id(ID *id, LibraryForeachIDData *data)
@@ -887,15 +889,27 @@ static void collection_object_cache_free(Collection *collection)
collection->flag &= ~(COLLECTION_HAS_OBJECT_CACHE | COLLECTION_HAS_OBJECT_CACHE_INSTANCED);
BLI_freelistN(&collection->runtime.object_cache);
BLI_freelistN(&collection->runtime.object_cache_instanced);
}
void BKE_collection_object_cache_free(Collection *collection)
{
collection_object_cache_free(collection);
LISTBASE_FOREACH (CollectionParent *, parent, &collection->runtime.parents) {
collection_object_cache_free(parent->collection);
}
}
void BKE_collection_object_cache_free(Collection *collection)
void BKE_main_collections_object_cache_free(const Main *bmain)
{
collection_object_cache_free(collection);
for (Scene *scene = bmain->scenes.first; scene != NULL; scene = scene->id.next) {
collection_object_cache_free(scene->master_collection);
}
for (Collection *collection = bmain->collections.first; collection != NULL;
collection = collection->id.next) {
collection_object_cache_free(collection);
}
}
Base *BKE_collection_or_layer_objects(const Scene *scene,

View File

@@ -1431,6 +1431,8 @@ void BKE_main_collection_sync_remap(const Main *bmain)
/* On remapping of object or collection pointers free caches. */
/* TODO: try to make this faster */
BKE_main_collections_object_cache_free(bmain);
for (Scene *scene = static_cast<Scene *>(bmain->scenes.first); scene;
scene = static_cast<Scene *>(scene->id.next)) {
LISTBASE_FOREACH (ViewLayer *, view_layer, &scene->view_layers) {
@@ -1447,14 +1449,12 @@ void BKE_main_collection_sync_remap(const Main *bmain)
view_layer_bases_hash_create(view_layer, true);
}
BKE_collection_object_cache_free(scene->master_collection);
DEG_id_tag_update_ex((Main *)bmain, &scene->master_collection->id, ID_RECALC_COPY_ON_WRITE);
DEG_id_tag_update_ex((Main *)bmain, &scene->id, ID_RECALC_COPY_ON_WRITE);
}
for (Collection *collection = static_cast<Collection *>(bmain->collections.first); collection;
collection = static_cast<Collection *>(collection->id.next)) {
BKE_collection_object_cache_free(collection);
DEG_id_tag_update_ex((Main *)bmain, &collection->id, ID_RECALC_COPY_ON_WRITE);
}