BMain locking during the 'after liblink' doversion #92333

New Issue

Bastien Montagne · 2021-10-19T09:49:29+02:00

Bastien Montagne commented

2021-10-19 09:49:29 +02:00

in 6a850f3cc8, Main.is_locked_for_linking was added, and is currently only used in versioning code (both before and after liblink), to prevent adding or deleting IDs from Main.

However, exceptions have already been added to allow creation of workspaces and collections in that context. And {D12722#inline-108272} adds nodetrees to the exceptions.

Problem

While preventing before-liblink doversion to add or remove any ID makes total sense, I do not understand why after-liblink doversion should have this constraint.

To me the bug fixed by the commit is a bug in the (removed) doversion code, not something inherently forbidden at this stage?

Proposed Solution

Remove that lock check altogether in after-liblink doversion.

I think however that we could use that lock on all other Mains besides the one being processed (to ensure doversion code does not create/delete things outside its own scope). And do-version code there should be smart enough to check when it runs on the local main, or a lib main, and adapt its code accordingly.

Another thing to fix is in BKE_libblock_alloc, which should assign main->curlib to the newly created ID's id->lib (and tag it as indirectly linked). That way if doversion needs to create an ID in a library context, that new ID is properly created as a 'virtual' indirectly linked data, and does not break Main consistency (even if doing stupid things like generating default brushes for a lib main). Done in 7681326acd

in 6a850f3cc8, `Main.is_locked_for_linking` was added, and is currently only used in versioning code (both before and after liblink), to prevent adding or deleting IDs from Main. However, exceptions have already been added to allow creation of workspaces and collections in that context. And {[D12722](https://archive.blender.org/developer/D12722)#inline-108272} adds nodetrees to the exceptions. Problem ---------- While preventing before-liblink doversion to add or remove any ID makes total sense, I do not understand why after-liblink doversion should have this constraint. To me the bug fixed by the commit is a bug in the (removed) doversion code, not something inherently forbidden at this stage? Proposed Solution --------------------- Remove that lock check altogether in after-liblink doversion. I think however that we could use that lock on all other Mains besides the one being processed (to ensure doversion code does not create/delete things outside its own scope). And do-version code there should be smart enough to check when it runs on the local main, or a lib main, and adapt its code accordingly. ~~Another thing to fix is in `BKE_libblock_alloc`, which should assign `main->curlib` to the newly created ID's `id->lib` (and tag it as indirectly linked). That way if doversion needs to create an ID in a library context, that new ID is properly created as a 'virtual' indirectly linked data, and does not break Main consistency (even if doing stupid things like generating default brushes for a lib main).~~ Done in 7681326acd

Bastien Montagne commented

2021-10-19 09:49:29 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'