RFR: 8188858: Caching latestUserDefinedLoader() results in ObjectInputStream.readObject()

[Peter Levart]

On 10/16/2017 11:02 AM, Peter Levart wrote:
> For example:
> - let public readObject() / readUnshared() entry and exit points just 
> clear the cached loader (set it to null).

An alternative would be for entry point to save and clear the cached 
loader while exit point would restore / clear it if it is from correct 
thread / when the call was not nested. Like the following:

public Object readObject() {
     CachedLoader outerCL = cachedLoader;
     cachedLoader = null;
     try {
         ...
     } finally {
         if (outerCL == null || outerCL.thread == Thread.currentThread()) {
             // restore/clear cached loader when nested/outer call ends
             cachedLoader = outerCL;
         }
     }
}

with resolveClass() fragment repeated here for comparison:

           CachedLoader cl = cachedLoader;
           Thread curThread = Thread.currentThread();
           ClassLoader loader;
           if (cl == null) {
               loader = latestUserDefinedLoader();
               cachedLoader = new CachedLoader(loader, curThread);
           } else if (cl.thread == curThread) {
               loader = cl.loader;
           } else {
               // multi threaded use
               loader = latestUserDefinedLoader();
           }

           // and then...
           return Class.forName(name, false, loader);


There are all sorts of races possible when called concurrently from 
multiple threads, but the worst consequence is that the loader is not 
cached. I also think that even in the presence of races, the 
cachedLoader is eventually cleared when all calls to OIS complete. I 
couldn't think of a situation where such cached loader would remain 
hanging off the completed OIS because of races.

Well, there is one such situation but for a different reason. For 
example, if an OIS subclass is constructed solely to override 
resolveClass method to make it accessible to custom code (for example, 
make it public and call super.resolveClass()) in order to provide a 
utility for resolving classes with the default OIS semantics, but such 
OIS instance is never used for deserialization itself 
(readObject()/readUnshared() is never called).

To solve this problem, resolveClass() logic, including lazy caching, 
should be moved to a private method (resolveClass0()) with protected 
resolveClass() treated like public readObject()/readUnshared() with 
before/after treatment of cached loader around delegation to 
resolveClass0(). All OIS internal uses of resolveClass() should then be 
redirected to resolveClass0().

Regards, Peter