Modify

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#10203 closed enhancement (invalid)

weak references in a list instead in weakhashmap

Reported by: anonymous Owned by: team
Priority: normal Milestone:
Component: Core Version:
Keywords: Cc:

Description

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.

Attachments (0)

Change History (4)

comment:1 Changed 7 years ago by bastiK

Resolution: invalid
Status: newclosed

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

comment:2 Changed 7 years ago by 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();
}

Last edited 7 years ago by Don-vip (previous) (diff)

comment:3 Changed 7 years ago by 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...)

comment:4 Changed 7 years ago by Don-vip

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.