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

Kazunori Ogata OGATAK at jp.ibm.com
Fri Oct 13 12:36:55 UTC 2017


Hi Alan,

Alan Bateman <Alan.Bateman at oracle.com> wrote on 2017/10/12 22:17:48:

> This is better but it still not safe. You'll have to atomically set/get 
> the cachedLoader or put it into a thread local to ensure that 
> resolveClass picks up the loader cached by the current thread. A thread 
> local could work too although (needs study) it might need a reference to 

> the OIS to guard against nested deserialization with a different stream.

Thank you for your review. I fixed the code that does not read the 
cachedLoader atomically when the code tries to update it.

Updated webrev: http://cr.openjdk.java.net/~horii/8188858/webrev.02/


The updated code does not use atomic CAS or ThreadLocal.  This code can 
race when the cachedLoader is null, but I think it works correctly because 
a pair of a thread and a class loader (stored in a CachedLoader) is always 
correct regardless of the value of the cachedLoader field, and the thread 
that writes the cachedLoader last resets it to null.  The thread that 
failed to cache a loader simply calls latestUserDefinedLoader(), which is 
the same behavior as the original code.

Considering that multi-thread use of an OIS is rare, I don't want to add 
an overhead of CAS to update the cachedLoader, especially on the POWER 
platform because CAS has high overhead.  Caching loaders in a ThreadLocal 
in each OIS can be more thread safe, but I'm not sure if its memory 
overhead is worth doing so for the rare (and correctly working) case.


Regards,
Ogata



More information about the core-libs-dev mailing list