Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WeakEventBus that uses a weak reference to registered objects #807

Closed
gissuebot opened this issue Oct 31, 2014 · 27 comments
Closed

WeakEventBus that uses a weak reference to registered objects #807

gissuebot opened this issue Oct 31, 2014 · 27 comments

Comments

@gissuebot
Copy link

gissuebot commented Oct 31, 2014

Original issue created by goo...@sa.me.uk on 2011-11-29 at 10:27 PM


EventBus maintains a strong reference to registered objects, and objects can't easily be wrapped in a WeakReference because that would need to @Subscribe to the same events.

WeakEventBus would use a WeakReference to refer to registered objects and unregister them automatically using a ReferenceQueue.

@gissuebot
Copy link
Author

Original comment posted by wasserman.louis on 2011-11-30 at 05:03 AM


Could you explain an actual real-world use case?

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by goo...@sa.me.uk on 2011-11-30 at 07:35 AM


I'm using an EventBus to handle locale changes at runtime. If I subscribe every new Action object (so that its name can be changed automatically) then they stay referenced by the EventBus forever.

@gissuebot
Copy link
Author

Original comment posted by wasserman.louis on 2011-11-30 at 08:49 PM


Is there a point where you can explicitly use the unregister() method of EventBus?

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by goo...@sa.me.uk on 2011-11-30 at 09:04 PM


They're added to a temporary JPopupMenu. Every Action has a strong reference to JPopupItem via the PropertyChangeListener so it never calls removePropertyChangeListener(). I could try having the Action use FinalizableWeakReferences to its listeners.

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by goo...@sa.me.uk on 2011-11-30 at 09:38 PM


This isn't very practical as PropertyChangeSupport makes a distinction between PropertyChangeListener and PropertyChangeListenerProxy. I'd need to have two versions of the weak references as the implementation of PropertyChangeListenerProxy relies on PropertyChangeSupport filtering the events.

I've tried implementing a WeakEventBus but it's not currently working (although a proxy listening object with a WeakReference to the original listening object worked).

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by goo...@sa.me.uk on 2011-11-30 at 09:48 PM


The implementation of AbstractButton is careful not to maintain a strong reference to the Action when adding itself as a PropertyChangeListener of the Action.

It uses a proxy object that only has a strong reference to the AbstractButton (JMenuItem). If I weakly reference that object from the Action then nothing will have a strong reference to it and it could disappear while it should still exist.

GC Root -> EventBus -> Action -> ButtonActionPropertyChangeListener -> JMenuItem
GC Root -> JPopupMenu -> JMenuItem -> Action

If I use a weak reference for Action -> ButtonActionPropertyChangeListener then it could drop the ButtonActionPropertyChangeListener while the JPopupMenu still exists:
GC Root -> EventBus -> Action
GC Root -> JPopupMenu -> JMenuItem -> Action

There's then no way for the Action to call the JMenuItem when its properties are changed and it would mistakenly assume it has no listeners so unregister itself from the EventBus.

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by goo...@sa.me.uk on 2011-11-30 at 09:55 PM


(My WeakEventBus is working, I just didn't have the right log level to see it doing the cleanup)

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by goo...@sa.me.uk on 2011-11-30 at 10:26 PM


Actually, ButtonActionPropertyChangeListener doesn't do what its comments imply it is supposed to do because AbstractButton maintains a reference to it. There's a lot of "don't use an anonymous subclass" comments in the source but then it stores a reference to "this".

For AbstractButton (JMenuItem) <-> Action they need strong references to each other in order to be able to call their respective listener events.

JPopupMenu doesn't clear out its items when the menu is hidden because it's reusable, so the reference from the EventBus becomes a problem as it keeps these two components alive.

I can't use PopupMenuListener from the Action to detect when the menu is visible/invisible because it never sees the JPopupMenu, only the proxy object for JMenuItem.

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by goo...@sa.me.uk on 2011-11-30 at 11:33 PM


If I handle PopupMenuListener.popupMenuWillBecomeInvisible() I can set the Action to null on all the AbstractButtons and then have the Action detect when there are no PropertyChangeListeners, but that's very messy and it relies on the JPopupMenu clearing out the references from all its components when it's hidden.

@gissuebot
Copy link
Author

Original comment posted by wasserman.louis on 2011-12-02 at 06:20 PM


I agree that this is not a use case that EventBus currently handles, but I'm not yet sure that weak references are the only way, or the best way, to address this use case.


Status: Triaged

@gissuebot
Copy link
Author

Original comment posted by fry@google.com on 2011-12-05 at 07:00 PM


(No comment entered for this change.)


Labels: Type-Enhancement

@gissuebot
Copy link
Author

Original comment posted by cbiffle@google.com on 2011-12-09 at 06:14 PM


This reminds me of NSNotificationCenter in Cocoa, which has always used weak references (and recently switched to proper zeroing weak references, rather than crashing weak references). NSNotificationCenter influenced the design of EventBus from my previous experience in Objective-C. EventBus probably would have used weak references from the start, except that the application I built it for wound up with data processing objects that were only retained by the bus.

I'm not sure a bus-wide weak reference policy is the right approach, though it would work. Perhaps the right thing would be to have object-by-object control at the register level?

@gissuebot
Copy link
Author

Original comment posted by fry@google.com on 2011-12-10 at 04:24 PM


(No comment entered for this change.)


Labels: Package-EventBus

@gissuebot
Copy link
Author

Original comment posted by limpbizkit on 2012-01-31 at 07:58 PM


One solution is to create your own subscriber class that takes care of the weak reference:

public class WeakExample {
    public static class HeavyListener {
        public void receiveEvent(Object event) {
            System.out.println(event);
        }
    }

public static class WeakEventDispatcher {
    private final WeakReference<HeavyListener> ref;
    private final EventBus eventBus;
    WeakEventDispatcher(HeavyListener listener, EventBus eventBus) {
        this.ref = new WeakReference<HeavyListener>(listener);
        this.eventBus = eventBus;
    }
    @Subscribe public void subscribe(Object event) {
        HeavyListener l = ref.get();
        if (l == null) {
            eventBus.unregister(this);
        } else {
            l.receiveEvent(event);
        }
    }
}

public static void main(String[] args) {
    HeavyListener heavyListener = new HeavyListener();
    EventBus eventBus = new EventBus();
    eventBus.register(new WeakEventDispatcher(heavyListener, eventBus));
    eventBus.post("hello");
}

}

This is simple and predictable. And if you're willing, it allows you to use a ReferenceQueue to deregister the listener eagerly.


Status: Fixed

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by goo...@sa.me.uk on 2012-01-31 at 08:04 PM


I don't see how this fixes the issue because you're assuming that I want to subscribe to Object and not arbitrary event types (for which I'd need a separate dispatcher).

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2012-02-01 at 06:29 AM


I actually agree it was a misfire to mark it "fixed", Jesse.

I really want to know if this is something that a lot of users would find themselves wanting to do, and am still receptive to adding something like an EventBus.weakRegister() if so.


Status: Triaged

@gissuebot
Copy link
Author

Original comment posted by b.diedrichsen on 2012-02-09 at 01:06 PM


I have been working on an implementation of a similar mechanism before I stumbled upon the guava one. Basically it does the same thing but uses weak references to the registered objects. The motivation behind it was that in a spring environment I wanted to use bean post processor to register the listening beans. Since a bean might have a different scope and might be short living I don't want to end up having all the objects prevented from being garbage collected. And I do not want to leave the burden of unregistering objects to other developers. I like the idea of the contract: Add the listener and as long as it exists it will receive events. If the listening objects dies then it will be removed from bus. It seems that this is kindof easy to handle from a user perspective (but a little tricky to implement, especially in concurrent environment).

@gissuebot
Copy link
Author

Original comment posted by fry@google.com on 2012-02-16 at 07:17 PM


(No comment entered for this change.)


Status: Acknowledged

@gissuebot
Copy link
Author

Original comment posted by jborden on 2012-04-04 at 07:00 PM


The issue I'm facing is a case I have an instance of a Cache that is Session Scoped using Guice that needs to handle events fired by a Singleton Scoped mechanism.

It seems that Guice does not provide a mechanism through which I can handle the event of the Session Scoped instance becoming unbound ( http://code.google.com/p/google-guice/issues/detail?id=62 ). Consequentially, I cannot explicitly unregister my Session Scoped listener from the event bus.

It seems that I should be able to get by using the workaround presented in Comment 14 but it seems that adding something like an EventBus.weakRegister(...) method would be ideal for this scenario.

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2012-05-30 at 07:43 PM


(No comment entered for this change.)


Labels: -Type-Enhancement, Type-Addition

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2012-06-22 at 06:16 PM


(No comment entered for this change.)


Status: Research

@gissuebot
Copy link
Author

Original comment posted by tadas.subonis on 2012-07-22 at 10:05 AM


Is any work is being done for this feature?

@gissuebot
Copy link
Author

Original comment posted by b.diedrichsen on 2012-10-23 at 08:50 AM


It has been a while since nothing happened here regarding that feature. Since I was in desperate need for a solution that uses weak references I created my own. We have been using it in production for half a year now and it works perfectly. It is well documented and works very similar to guavas event bus (although its a bit faster).

You can check it out here: https://github.com/bennidi/mbassador

Sorry, for that "cross marketing" here but I see that other people have the same needs as I do and are thus not able to use the guava version of event bus. Besides, I love the guava library and have been using it in production for a while now.

@gissuebot
Copy link
Author

Original comment posted by vojtek44 on 2012-12-10 at 04:07 PM


Well, I find myself to need this feature too - I register new beans from Spring IoC automatically in event bus and it would be great if EventBus would not consider them when they are out of their scope. In my case the solution from #14 is good enough, however a weakRegister() method would be much better.

@gissuebot
Copy link
Author

gissuebot commented Nov 1, 2014

Original comment posted by r...@bogocorp.com on 2013-03-02 at 12:05 AM


The solution in #14 is interesting, but to truly be awesome the WeakEventDispatcher class would take a plain Object as the heavy listener, inspect it for @Subscribe methods and dispatch events as appropriate. And yes, a ReferenceQueue for quicker cleanup would be nice.

But, having the WeakEventDispatcher @Subscribe to Object events and then potentially ignore them means that DeadEvents won't work.

That, I think, can only be solved by building weak subscriptions directly into EventBus.

@gissuebot
Copy link
Author

Original comment posted by progoth on 2013-08-28 at 04:28 PM


I'm using Guice to register all my injections with the eventbus, and using Providers to create (swing) dialogs that communicate with a database controller using guava eventbus. For dialogs to not need eventbus cleanup I'd have to write and maintain reset methods for each to go back to their default state, and every dialog (or opened dialog, if using lazy vals with Providers) would stay in memory for the life of the application.

Since my dialogs are confined to a few controllers, it's not out of the question to wrap their usages in something like

def using[T](prov: Provider[T])(f: T => Unit) {
    val t = prov.get
    ultimately(eventBus.unregister(t))(f(t))
}

which I may do. I'll probably take a look at MBassador, at least for some projects where there are few classes that need strong references and those can be specified with class annotations (adding weakRegister to eventbus would require guice to register everything weakly and then I'd have to maintain references to some classes manually).

@netdpb
Copy link
Member

netdpb commented Aug 12, 2019

We are not actively working on EventBus anymore. There are modern alternatives, including RxJava, that are more actively maintained.

@netdpb netdpb closed this as completed Aug 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants