- I improved the AttributeSource to no longer use a LinkedList for the implemented interfaces of an Impl
- I am not really happy about this, but SoftReference was also wrong. After thinking more than 24 hrs about it, I have no good solution to prevent the static map from sitting on classes forever. The fix here is: We use MethodHandles only for attributes that were loaded by Lucene's own classloader. Really foreign ones (e.g. from Solr plugins) will solely use reflective mode to create new instances of attributes.
Here the explanation:
The problem is that we originally had a map Attribute -> AttributeImpl, which was weak for keys and values. By this the classloader can safely remove loaded classes, because we don't strongly reference the attributes nor the implementations. The values must be weak, too, otherwise the impl class indirectly strong references the interface (because its class object implements the interface). As Weak maps rely on the fact that the key get GCed, this will never happen, because of the strong value reference.
This all went fine with the pure reflective approach. But when using method handles, we can no longer put them as Weak values into the map. Because nothing has a strong reference to the MethodHandle, its cleaned up asap. So we have the reflective access again and again. If we make the MethodHanlde a strong reference, we have the same problem like above: MethodHandle refers to its class (the AttributeImpl) and that one refers to the interface -> the key can never GCed.
The workaround here is: The cache handles both cases (MethodHandles and reflective). The MethodHandles are strong references and are only used for Lucene's own classloader. So we cannot sit on foreign classes from plugins. Foreign interface implementations are handled like before.
The fast path in createAttributeInstance() is now:
- check cache
- if cache contains MethodHandle invoke it directly (this is faster than before, because no wek ref dereferring)
- if cache contains a Reference object (pointing to our the impl class), we use reflective: Class#newInstance() [same as in earlier Lucene versions]
- if the cache does not have an entry, we load the class as always
- if classloader is our own, we resolve the method handle and store it as hard reference in the weak map and execute it
- otherwise we put a WeakReference to the class for later reflective access into the map.
We have a test: newAttributeImpl in BaseTokenStreamTestcase chooses between all 4 AttributeFactories: DEFAULT (MethodHandles and Reflective), Reflective only, the packed impl, and Token's attribute factory.