Message11521

Ok, given the map is thread safe, the question about cache coherency is probably not an issue. I see the intended design here as relying on the map's thread safety rather than synchronising PyType as a whole. I guess we're trying to avoid holding a lock on a global (PyType.class) for longer than is necessary in this hot part of Jython.

As implemented, though, we leave completing the job until after the put(), and therefore unsynchronised (if I've understood it correctly). This was not so when the lock was on the PyType class. I assume the motivation for this two-phase construction is to deal with recursive reference. And you can't complete it before the put() for that reason. (Parallel: when loading Python modules we enter them in sys.modules *before* initialising them in case they refer to themselves.)

Is there a case for a map of part-constructed PyTypes that we are allowed to use in constructing other PyTypes but not in resolving regular accesses? Or is that just another thinking error?

Weak references make the problem occur more frequently because we repeat the faulty construction (I think). I appreciate the question of class unloading, but as written it seems like every time we stop using the exposed version of a class momentarily, we're going to pay for a new PyType. (Every time but the last, that is.) I think this is behind the comment in the docs for MapMaker here: https://google.github.io/guava/releases/19.0/api/docs/com/google/common/collect/MapMaker.html#weakValues(), still recommending softValues() despite having removed that method.

Is ClassValue what we ought to be using here? I'm pleased to find that it is available from Java 7, not 8 as I thought. You'd still have to complete construction before making it visible to attribute access.

Also, JLS 17.4.4: "Programmers do not need to reason about reorderings to determine that their code contains data races."