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.
