Second Code Review for WeakReference leak in the Logging API (6942989)
Daniel D. Daugherty
daniel.daugherty at oracle.com
Wed Jun 23 17:38:20 PDT 2010
On 6/23/2010 12:38 PM, Jeremy Manson wrote:
> Hi Daniel,
> I'm sorry I missed this (I heavily filter these lists, and check rarely).
This time I specifically left you on the "To" list rather than
editing down to just the list aliases.
> My main feeling is that you are missing a good bet by not
> reconstructing the Hashtable in LogManager and the ArrayList in Logger
> every so often when you remove the loggers. In a test case where
> there are a LOT of short-lived loggers, the backing array for the
> Hashtable can get very big. It is permanent, and doesn't go anywhere.
> You can end up with a lot of extra memory lying around that way.
It's possible that is a good bet. However, that wasn't mentioned
as one of the issues in either bug report (I think) so I didn't
write a test for that.
> Specifically, when I didn't reconstruct those data structures, the
> test case listed in the bug (where it just creates lots and lots of
> anonymous loggers) killed the Java instance with an OOM, even if I
> *did* clean up the weakreferences to the loggers.
I'll create a variant of the anon logger test that I put in the
changeset and checkout if I can kill the Java instance with an OOM.
I'll keep you posted.
> I'm assuming you have a customer waiting for this - if that is similar
> to their usage pattern, this fix may not fix their problem.
Thanks for the heads up. The JDK6 version of the fix hasn't been tested
by the customer yet so you might be right.
> You obviously don't want to rebuild those structures every time,
> though. What I did in my change was to reconstruct the backing data
> structures every time ~as many loggers were collected as were present
> in the data structure.
Yup. I caught that part of the rebuild algorithm. It's just that the
reason for doing the rebuild didn't jump out at me.
> On Fri, Jun 18, 2010 at 12:25 PM, Daniel D. Daugherty
> <daniel.daugherty at oracle.com> wrote:
>> I have a new version of my fix for the WeakReference leak in the
>> Logging API done. This version uses ReferenceQueues; thanks to Eamonn
>> McManus, Jeremy Manson and Tony Printezis for their insights on using
>> ReferenceQueues. Here's a pointer to Tony's paper for background info:
>> This version also has limits on the number of dead Loggers that are
>> cleaned up per call; thanks to Alan Bateman for politely pushing me in
>> that direction.
>> The webrev is again relative to OpenJDK7, but the bug is escalated so
>> the fix will be backported to the JDK6-Update train. So again, I'll
>> need a minimum of two code reviewers.
>> Here is the URL for the webrev:
>> Thanks, in advance, for any reviews.
More information about the hotspot-runtime-dev