Potential fatal errors in box-related loops that fire traps

[Charles Goodwin]

e.g. from fireVisibleTraps:

        for (int i=0; i<treeSize(); i++) {
            Box b = getChild(i);
            if (b.test(DISPLAY))
             trapchain = b.fireVisibleTraps(val, trapchain);
        }

Originally Adam had a concept of 'nextSibling' and 'prevSibling'. It's obvious that he did this because the trap firing may alter the state of the box tree, thus the current loop must be flexible to deal with this.

[mikeyg (mike-goodwin)]

Its not a race conditon, as js and all box operations are performed in one thread.

What it does mean that in certain sensitive points vexi code could potentially get executed with bad logic or core exceptions may get thrown. This could be potentially confusing for a vexi application developer.
.
One solution would be to lock the box or possibley a box tree at certain points, preventing add/remove child operations. Ideally we would just lock the box before entering the loop and unlock it after.

[Charles Goodwin (charlesgoodwin)]

Made low importance because, well, we've lived with it for a long time without problems and it only applies to a very small number of corner cases (when you are moving sibling boxes from within a trap on particular properties on a box).

[Charles Goodwin (charlesgoodwin)]

Bah, I scratched the itch because it took <10 mins to fix. Just went through Box.jpp, finding loops over children, and added a simple lock around it:

boolean lock = !test(LOCK_CHILDREN);
if (lock) set(LOCK_CHILDREN);
// loop
if (lock) clear(LOCK_CHILDREN);

The lock is required because LOCK_CHILDREN may already have been set in very complicated cases.