Code Review for WeakReference leak in the Logging API (6942989)
Alan Bateman
Alan.Bateman at oracle.com
Fri Jun 11 13:39:48 PDT 2010
Daniel D. Daugherty wrote:
> :
> Either of those schemes would be fine, but not for this fix and
> not without a good reason to do so. The theory is that there
> shouldn't be too many Logger objects in a normal system and
> once they have been added, then this fix doesn't come into play.
> I would be surprised if a real system had more than 100 Logger
> objects.
I'm sure this is mostly true but there will be extreme cases where it
might be a problem. I remember Mandy discovering that AWT has a Logger
per class so you might want to check that the changes don't impact
startup - as I recall all the AWT Loggers are created in static
initializers (discussed on awt-dev a while back).
> :
>
> So would "kidsCleanupSentinel" be better? (I actually rather liked
> the implied humor of kids and markers... :-))
It is funny. I don't have a strong preference. One idea is to give it a
simple name and declare it close to the method so that it is clear that
this is the only place where it is used (your call).
>
> :
> By using "jmap -histo:live" I'm doing a targeted sample of just
> WeakReference leaks and that's what I want for this regression
> test. If I was creating a general "does the Logging API leak
> memory?" test, then the pure Java test with a small/limited heap
> size would be a good approach.
I understand but just concerned that these multi-process tests have a
tendency to be problematic. A simple pure-java test that specifies a
small maximum heap size in the @run tag may prove to be more robust.
>
> Can I put you down as a "thumbs up"?
Yes, I'm okay with what you have assuming that it doesn't impact AWT
startup. Also Éamonn makes a good point (so maybe create a bug so that
they code can be replaced/rewritten in jdk7?).
-Alan.
More information about the serviceability-dev
mailing list