weak references in a list instead in weakhashmap

[anonymous]

are you sure your listeners list in Main.java

private static final List<WeakReference<ProjectionChangeListener>> listeners = new ArrayList<>();

is working as expected?

for this usecase a WeakHashMap or some third party WeakList are intended.

I would make a bet that orphan (weakly reachable) objects are not garbage collected from a List.

[bastiK]

Don't worry, WeakReference in a List works just fine.

[anonymous]

after playing a while with weak refs I got the idea of them. this code to proofs your answer.

public class Client {

        public static void main(String[] args){
                Client c = new Client();
                c.initListener();
                c.callListener();
        }

        private void initListener() {
                ProjectionChangeListener listener = new ProjectionChangedListenerImpl();
                JosmMain.addProjectionChangeListener(listener);
                //local variable listener not used any more and will be gc'ed
        }
        
        public void callListener(){
                while(true){
                        JosmMain.fireProjectionChanged();
                }
        }
}


public class JosmMain {

        private static final List<WeakReference<ProjectionChangeListener>> listeners = new ArrayList<>();
        
        public static void addProjectionChangeListener(ProjectionChangeListener listener) {
        if (listener == null) return;
        synchronized (JosmMain.class) {
            for (WeakReference<ProjectionChangeListener> wr : listeners) {
                // already registered ? => abort
                if (wr.get() == listener) return;
            }
            listeners.add(new WeakReference<>(listener));
        }
    }

    /**
     * Removes a projection change listener.
     *
     * @param listener the listener. Ignored if <code>null</code>.
     */
    public static void removeProjectionChangeListener(ProjectionChangeListener listener) {
        if (listener == null) return;
        synchronized(JosmMain.class){
            Iterator<WeakReference<ProjectionChangeListener>> it = listeners.iterator();
            while (it.hasNext()){
                WeakReference<ProjectionChangeListener> wr = it.next();
                // remove the listener - and any other listener which got garbage
                // collected in the meantime
                if (wr.get() == null || wr.get() == listener) {
                    it.remove();
                }
            }
        }
    }
    
    public static void fireProjectionChanged() {

            synchronized(JosmMain.class) {
                Iterator<WeakReference<ProjectionChangeListener>> it = listeners.iterator();
                while (it.hasNext()){
                    WeakReference<ProjectionChangeListener> wr = it.next();
                    ProjectionChangeListener listener = wr.get();
                    if (listener == null) {
                        System.out.println("listener null");
                        it.remove();
                        continue;
                    }
                    listener.projectionChanged();
                }
            }
    }
}

public class ProjectionChangedListenerImpl implements ProjectionChangeListener{

        @Override
        public void projectionChanged() {
                System.out.println("Projection changed");
        }

}

public interface ProjectionChangeListener {
        void projectionChanged();
}

[bastiK]

Actually you are right that a WeakHashMap would be slightly better because it prevents empty WeakReference objects from piling up in the list. However, this memory leak is negligible in this case and does not need to be fixed. (If you make a patch, I will apply it...)

[Don-vip]

If you provide a patch please think about giving us a username ("patch by anonymous" looks strange in the changelog)