Code Review for WeakReference leak in the Logging API (6942989)

[Daniel D. Daugherty]

On 6/11/2010 2:39 PM, Alan Bateman wrote:
> 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).

Mandy posted numbers just before this post in the e-mail thread.
I still don't consider those numbers to be "a lot" of Loggers,
but the point is likely moot depending on the results of my
investigation into ReferenceQueues and Jeremy Manson's fix for
this same problem.


>
>> :
>>
>> 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).

Hold that thought since this field will likely be going away...


>
>>
>> :
>> 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.

No argument about robustness, but I think I've done a pretty
good job of guarding the test against spurious failures. And
if it does fail strangely there are good diagnostics (IMHO).

I have different goals for the test. I only want to catch
these two fish in this net. The pure Java test may give me
more potential issues to investigate or rule out and I don't
have time for that right now.


>
>>
>> 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?).

Stay tuned. It may be rewritten today. :-)

Dan