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

Kazunori Ogata OGATAK at jp.ibm.com
Fri Oct 27 13:10:38 UTC 2017


Hi Alan,

Thank you for your review and comments.

I updated webrev based on your comments: 
http://cr.openjdk.java.net/~horii/8188858/webrev.05/


Alan Bateman <Alan.Bateman at oracle.com> wrote on 2017/10/26 19:31:53:

> I've read through webrev.03 and webrev.04 and they seem correct. Are 
> there are more comments on the implementation? I'd like to get someone 
> in the security area to review this before it is sponsored, the reason 
> is that original patches could be abused and this is security-sensitive 
> area.

I added/revised comments, though I'm not sure it is sufficient.  It is 
helpful if you could point out the changes that merely have insufficient 
comments.


> One comment, or suggestion, is to find a better name for the 
> cachedLoader field, esp. as its value may be a Thread or a CachedLoader 
> object. If CachedLoader were renamed to ReaderContext and cachedLoader 
> changed to something like currentReader then it might be a bit clearer. 
> I'm sure there are better names, the important thing is that future 
> maintainers can quickly understand the code.

Agreed. I renamed CachedLoader to ReaderContext, and also renamed other 
variables to make it easier to identify they are related to ReaderContext.


Regards,
Ogata



More information about the core-libs-dev mailing list