Revisiting this e-mail now that I've completed a first pass at a ReferenceQueue implementation.
On 6/11/2010 10:36 AM, 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.
The WeakReference subclass that I created manages weaks refs in LogManager.loggers (now named LogManager.namedLoggers), LogManager.LogNode.loggerRef and Logger.kids. I got the idea of managing LogManager.loggers from Jeremy's fix.
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.
I created a LogManager.drainLoggerRefQueueBounded() method based on Tony's paper: http://java.sun.com/developer/technicalArticles/javase/finalization/ and I'm calling it on the LogManager.addLogger() and the Logger.getAnonymousLogger(String) code paths.
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.)
There is only one LogManager so I don't have to worry about redoing the LogManager synchronization stuff. drainLoggerRefQueueBounded() isn't called from the context of a Logger object so I don't have to worry about a current Logger versus other Logger context collision.
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.
I did make the parent Logger reference a weak reference, but not because of loops. I did it because I didn't want to add a strong reference to the parent Logger and keep it from being GC'ed. Sorry, I still can't figure out what loops have to do with it.
Nevertheless, I think this approach is likely to be less complicated than the proposed one,
I'm still not convinced this approach is less complicated, but it is definitely a much better approach :-)
notably because there's no need to protect against infinite recursion;
No more need for protecting 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.
Definitely less performance concerns. I also only cleanup a set maximum of GC'ed Logger objects per call. Tony P's paper also recommended this and the drain*Bounded() name too.
Éamonn
Thanks again for the ReferenceQueue idea and for your thorough review. I'm running my new version through VM/NSK logging tests right now; it passed the SDK/JDK LOGGING_REGRESSION tests. I'll make one more sanity check code review after that and send the revised bits out for a re-review... Dan
