On Tue, 4 Jul 2023 21:00:30 GMT, Mandy Chung <mch...@openjdk.org> wrote:
>> src/java.base/share/classes/java/lang/invoke/MethodHandleProxies.java line >> 324: >> >>> 322: r = PROXY_LOOKUPS.get(intfc); >>> 323: } >>> 324: return r.get(); >> >> This doesn't look right to me. AFAICT the Lookup can be GC'd before we call >> `get`. >> >>> No, it wouldn't. PROXY_LOOKUPS.get(intfc) returns the WeakReference of the >>> lookup class which is strongly reachable in the stack frame. >> >> I'm not sure how the lookup class (`intfc` ?) is keeping the `Lookup` alive >> here? > > Sorry I re-read Chen's question and missing the last half of the sentence. > My reply was about: If the referent is not GC'ed, `r.get()` returns non-null > which will be returned to the caller. If the referent becomes weakly > reachable, `r.get()` will return null. > > OK. In any case, I was thinking to use a local variable to keep the lookup > object strongly reachable. I adjusted the patch to keep a class value of a > holder of `WeakReference` > ([ab3d8b2](https://github.com/mlchung/jdk/commit/ab3d8b2f086dfac4f8fda1ddce415270c0c53d2b)): > > > private static Lookup getProxyClassLookup(Class<?> intfc) { > WeakReferenceHolder<Lookup> r = PROXY_LOOKUPS.get(intfc); > Lookup lookup = r.get(); > if (lookup == null) { > // If the referent is cleared, create a new value and update > cached weak reference. > lookup = newProxyLookup(intfc); > r.set(lookup); > } > return lookup; > } Ok, thanks. This is similar to the solution I would have suggested. The only thing I'm unsure about is races. ClassValue allows multiple threads to race to compute the final value, but in the end only 1 is chosen that is returned to all threads. If multiple threads race to create a new proxy lookup here, they will all get a distinct lookup returned to them. I think this is okay? Though, it would mean we have less sharing of proxy classes. Maybe this code should synchronize on `r` to make sure only 1 lookup is created per interface. (Letting threads race to compute the value is also a possibility, but it also leads some threads to do work that is then discarded, and then the VM has to clean up the class they defined as well). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13197#discussion_r1252373506
