RunTimeTyped queries broken during static initialisation

[johnhaddon]

As described at GafferHQ/gaffer#4320 (comment), making RunTimeTyped queries during static initialisation yields incomplete results, and worse, bakes those incomplete results into RunTimeTyped's internal registry, meaning it will return incomplete results for the duration of the process. Affected functions are :

• RunTimeTyped::baseTypeIds(). Because we can't control static initialisation order, the registration for a base class may not have been made at the time a derived class is registered. So baseTypeIds() is implemented to populate the registry of bases lazily, the first time it is called for a given type. If called during static initialisation, it may bake an incomplete list of bases.
• RunTimeTyped::derivedTypeIds(). This uses the same strategy as baseTypeIds(), but is even more broken. A Python import can add to the list of derived types at any time, so there is no time at which we can bake the complete results into the registry.
• RunTimeTyped::inheritsFrom(). This is implemented using baseTypeIds().

Possible solutions :

1. Implement inheritsFrom() using baseTypeId() rather than baseTypeIds(), so it doesn't trigger the early baking. This might be sufficient for the Gaffer use case for now.
2. If baseTypeIds() doesn't reach RunTimeTypedTypeId while generating the result, don't store it and instead return a reference to an empty vector. A later call will successfully do the baking after initialisation is complete.
3. Don't bake/store the results at all. Always do all the work and return a new result instead of a reference into the registry. This is the only option that can truly solve the problem for derivedTypeIds(), but it does mean more repeated work/allocation. Looking at the existing uses, I think this is probably acceptable performance-wise, particularly if we also do 1). It would be an ABI break though, returning by value rather than reference.

Thoughts @danieldresser-ie, @andrewkaufman?

[danieldresser-ie]

The big advantage of 1) is that inheritsFrom could be used during initialization in a contex where it would actually yield correct results, but would currently result in baking partially completed results into baseTypeIds that could cause problems later? It seems like it could get a bit slower if implemented using baseTypeId() instead?

It seems like derivedTypeIds would just give incorrect results if things aren't initialized yet, and it would be better to give an error than to give incorrect results?

I suppose there is an option 4) Drop all the caches whenever a new type gets registered
That would still allow for good performance in the general case?

[johnhaddon]

Chatting to @danieldresser-ie just now, we tentatively concluded that :

• If inheritsFrom() or baseTypeIds() detects incomplete registration of the base class chain, they should throw.
• We suspect that there are no performance-critical uses for any of these methods, so removing the caching and returning newly computed results each time might be fine (subject to reviewing current uses).
• If performance is critical, then a feasible approach is to drop caches on new registrations, but return shared_ptr from the query methods to provide thread-safety if one thread queries and one registers at the same time.