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

[Daniel D. Daugherty]

On 6/11/2010 2:02 PM, Alan Bateman wrote:
> Eamonn McManus wrote:
>> I think an alternative approach to the one here would be to use a 
>> global ReferenceQueue and a subclass of WeakReference that has a 
>> pointer back to the Logger or LogNode that contains this 
>> WeakReference. Then, in the cases where you currently check for stale 
>> entries, you could simply poll the ReferenceQueue and visit any 
>> Logger or LogNode that it is telling you has a cleared WeakReference.
>> There are a couple of subtle points, though. First, you'll need to be 
>> careful about synchronization before clearing WeakReferences, since 
>> the Logger you're being told about isn't necessarily related to the 
>> current one. Logger.kids is protected by a global lock treeLock but 
>> LogManager.loggers is protected by the LogManager's lock, so when 
>> logManager1.addLogger detects that a logger belonging to logManager2 
>> has a stale WeakReference, it needs to drop the logManager1 lock and 
>> acquire the logManager2 one before scanning the reference list. (This 
>> means that addLogger has to change from being a synchronized method 
>> to have a big synchronized(this) block with a non-synchronized 
>> tail-end.)
>> Second, since apparently the user can introduce loops in the Logger 
>> hierarchy, the reference from the WeakReference subclass back to the 
>> parent Logger will itself need to be a WeakReference.
>> Nevertheless, I think this approach is likely to be less complicated 
>> than the proposed one, notably because there's no need to protect 
>> against infinite recursion; and the performance concerns should be 
>> less because you only look for cleared WeakReferences when you know 
>> they are there, and you know where to look.
>> Éamonn
> This make sense as reference queues are the right way to do this. As I 
> understand it, Dan is looking for a low-risk solution now and so maybe 
> the right thing is to get that out of the way and then spend time on a 
> better solution for jdk7.

I'll be investigating ReferenceQueues and Jeremy Manson's fix for
this problem today. It's entirely possible that we'll drop my fix
in favor of something better.

Stay tuned...

Dan