views:

287

answers:

3

In the constructor of my class, I map the current object (this), along with its key (a string entered as a parameter in the constructor) into a static LinkedHashMap so I can reference the object by the string anywhere I might need it later.

Here's the code (if it helps):

public class DataEntry {
    /** Internal global list of DataEntry objects. */
    private static LinkedHashMap _INTERNAL_LIST;

    /** The data entry's name. */
    private String NAME;

    /** The value this data entry represents. */
    private Object VALUE;


    /** Defines a DataEntry object with a name and a value. */
    public DataEntry( String name, Object value )
    {
        if( _INTERNAL_LIST == null )
        {
            _INTERNAL_LIST = new LinkedHashMap();
        }

        _INTERNAL_LIST.put( name, this );

        NAME = name;
        VALUE = value;
    }
}

The problem? Instances of this class won't get garbage collected when I'm done using them.

I'm just curious if there's a way to have instances of this class clean themselves up when I'm done using them without having to manually call a Remove() method or something each time (to remove its reference in the internal LinkedHashMap when I'm no longer using them, I mean).

+1  A: 

What you want to use seems to be a weak reference. The concept is that weak references are not strong enough to force an object not to be GC'ed. I don't have much experience with them but you can learn more here.

Morten Christiansen
+6  A: 

Make the values WeakReferences (or SoftReferences) instead. That way the values can still be garbage collected. You'll still have entries in the map, of course - but you can periodically clear out the map of any entries where the Weak/SoftReference is now empty.

Jon Skeet
Damn, you beat me to the weak references :)
Morten Christiansen
Was busy editing - WeakHashMap isn't really suitable as it's the keys which end up being weak, not the values. You'll effectively have to write something somewhat similar, but it probably doesn't need to be quite as powerful if you're only using it for one particular situation.
Jon Skeet
Ok thanks. Yeah I was afraid of that. I'll probably just stick with using Remove methods and do it the lazy way then. :P
Daddy Warbox
+5  A: 

Making an object visible to others before its constructor is complete is not thread safe.

It's not clear how the map is being used in this case, but suppose there's a static method like this in the class:

public static DataEntry getEntry(String name) {
  return _INTERNAL_LIST.get(name);
}

Another thread, running concurrently, can access a DataEntry while it is being created, and start to use an entry with an uninitialized VALUE. Even you reorder the code in the constructor so that adding the new instance to the map is the last thing you do, the JVM is allowed to reorder the instructions so that the object is added to the list first. Or, if the class is extended, the subclass initialization could take place after the object has been published.

If more than one thread accesses the interacts with the DataEntry class, you could have a concurrency bug that is platform dependent, intermittent, and very tough to diagnose.

The article, "Safe Construction," by Brian Goetz has more information on this topic.

Back to the original question: using WeakReference, as mentioned by others, is a good approach, but rather than iterating over every entry in the map, I'd recommend creating a wrapper for your values that extends WeakReference (it could be your DataEntry itself, or a helper), and queuing each reference in a ReferenceQueue. That way, you can quickly poll the queue for any collected entries, and remove them from the map. This could be done by a background thread (blocking on remove) started in a class initializer, or any stale entries could be cleaned (by polling) each time a new entry is added.

If your program is multi-threaded, you should abandon LinkedHashMap for a map from java.util.concurrent, or wrap the LinkedHashMap with Collections.synchronizedMap().

erickson
Thanks for the additional information. Multi-threading is a bridge I may eventually need to cross, but for now I'm just bumbling my way through the basics. Anyway I'll consider that alternate approach.
Daddy Warbox