javax.imageio.spi.ServiceRegistry.finalize() just calls deRegisterAll()

deregisterAll() just loops over the categoryMap clearing the internal SubRegistrys
which just notify providers they are now de-registered by calling onDeregistration.
None of the JDK ones do anything with that although they all register to receive notification.

        for (SubRegistry reg : categoryMap.values()) {
            reg.clear();
        }


The problem is that the SubRegistry instances hold a reference to the ServiceRegistry because the API onDeregistration which they need to call receives that as a parameter.
So this means putting the categoryMap in the Disposer means the ServiceRegistry will never become unreachable.

And that the entire reason for deregisterAll() is to call all the registered providers passing the now unreachable
ServiceRegistry. This probably wasn't a great idea even in a finalize() world but it a complete non-starter for a cleaner.


There's no alternative I see to requring the app to call deregisterAll() directly before discarding the ServiceRegistry
Fortunately I think the impact is limited as I will describe below.

The finalizable behaviour was important in older JDKs because there is a ServiceRegistry per-applet context (AppContext).
This is actually the IIORegistry sub-class of ServiceRegistry which does this.
Even today we create one per-AppContext (per ThreadGroup) and the API spec for IIORegistry states this.
However since the Applet API is gone, we can re-specify this to stop using AppContext.
In fact another post-Applet API removal task is the huge chore of getting rid of AppContext.
It permeates the implementation like SecurityManager did.

So at that point we will only ever have one ServiceRegistry in the JDK and it will be a singleton in a static field.
And hence no clean up doesn't matter.

But what about 3rd party ServiceRegistry sub-classes ?
It isn't clear that these need to be 'finalized' either. I think a single ServiceRegistry for the life of the VM is the norm.

Corpus found a couple and I found the source for one - the one that was 21/22 hits in corpus.
org.geotools.factory.FactoryRegistry extends javax.imageio.spi.ServiceRegistry
but when I found their current version which is in a different package it had a NOTE:
https://docs.geotools.org/latest/javadocs/org/geotools/util/factory/FactoryRegistry.html

NOTE: Java 9 Service Registry Incompatibility. Prior releases of GeoTools uses Java's built-in ServiceRegistry to manage instances for our plug-in system. In Java 9 the service registry was restricted to a limited number of imageio services and was no longer available for general use. We have introduced CategoryRegistry to take over instance management, in conjunction with the supported ServiceLoader discovery.

Yes, we had to do this for jigsaw : https://bugs.openjdk.org/browse/JDK-8068749

So people basically had used it as a general purpose registry, since it was, but now it isn't useful.
So now that likely the JDK is now the only user. external sub-classes aren't a big worry.

And in the the impact is basically that providers which do ask to be told when they are de-registered, won't hear about it.
But I don't think it is common either. None of the JDK ones do anything.

For the sake of discussion is there anything which could be done ?

We could create a Disposer which keeps a simple list of all providers that are added and request a deregistration notice.
This list would need to be updated as providers are added or removed.
This disposer could call their onDeregistration() method with 'null' for the ServiceRegistry and
we could re-specify that this could be null in some cases and hope that doesn't cause NPEs.
Or hide that behaviour behind an off-by-default System property.
So don't do the calling unless the app opts in because they are seeing resource leakage.
But then they might just get an NPE if they access the registry,
Not sure if this is worth it without an actual customer use case.
And even then we might be better off to tell them to call deregisterAll.

So my conclusion is to just delete the finalize() support.