Need reviewer for 6902010: (cl) Delay initialization of ClassLoader.parallelLoaders

Alan Bateman Alan.Bateman at Sun.COM
Tue Nov 17 20:06:57 UTC 2009


Rémi Forax wrote:
> :
> Hi Alan,
> I think you can remove the intermediary synchronizedMap when creating 
> loaderTypes
> and add a synchronized to isRegistered.
Indeed, that would be nicer - as you probably guessed, I was just trying 
to avoid changing the original code.

>
> You should add a private constructor with no parameter that throw an 
> AssertionError
> because this class is not intended to be instantiated.
> (you need the AssertionError because a private constructor can be 
> called from enclosing class).
As you point it out, I can do this but if some future maintainer is 
changing ClassLoader then there is a lot worse things they can do :-)

>
> Moreover, I don't think it's a good idea to synchronized on the class 
> (here ParallelLoaders.class)
> because this object is public, by example you can get a reference to 
> it using a java agent.
> I would prefer a synchronized block on loaderTypes.
>
Right, it would be cleaner to synchronize on loaderTypes (although there 
is no real defense against an agent as it can it can change the byte code).

Thanks Rémi for spending time on this.

-Alan.




More information about the core-libs-dev mailing list