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

[Alan Bateman]

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.

-Alan.