Concerned use of 'static int cache' can break multiple VM instances

[ewmailing]

This was discussed in #547. Even though we are spending a bunch of time on a separate issue in that discussion, I am concerned this one is still an issue, so I want to file it so it doesn't get lost.

In the generated code from coder.lua, there are places that static variables are used for cache purposes.

        static int cache = -1;
        TValue *slot = pallene_getstr(9, x9, tsvalue(&K->uv[16].uv), &cache);

My concern is that if these values are not constant for all cases except maybe for some global one-time initialization, this could cause bugs and break user programs if they use multiple Lua VMs in their program. I don't fully understand what is being cached and the side effects of everything, but it raises alarm bells for me.

I was specifically thinking of Lua Lanes, which will have multiple instances of Lua VMs running concurrently across multiple threads. But I am concerned that even having just two different VMs that you run one after another could still potentially lead the second VM instance to pick up the static int cached value set from the prior first VM run instance, and maybe that cached value is now wrong for this second instance.

[hugomg]

This inline cache is meant to remember the key's position in the hash table. Normally, a hash lookup might need to look at more than one hash slot until it finds the item it's looking for. The cache is meant to speed this up so we start the search in the slot that is likely to hold the key we're looking for.

The slot is only effective if the next time around the table has the same internal structure. That is, it has the same set of keys and they were inserted in the same order. This is fairly common, e.g. tables created by the same function likely have the same internal structure. If the cache is invalid (because it's out of bounds or because the slot doesn't have the key we're looking for) then it ignores the cache and does a regular hash table lookup.

In theory, invalidating the cache shouldn't cause the program to crash, only to make it run slower. But threading is tricky and I'd believe you if we found evidence that it's causing a crash. In any case it shouldn't be too difficult to test this, by editing the getstr function so that it ignores the cache.