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