Registering Event Bus in Constructor

[fid...@gmail.com]

If you register an object on the event bus within the constructor for that object can another thread call subscribers on that object before the first thread has returned from the constructor?

private class Test{
  private final Object somethingThatShouldNotBeNull;
  public Test(EventBus eventBus){
    eventBus.register(this);
    // other stuff happens
    this.somethingThatShouldNotBeNull = new Object();
  }
 
  @Subscriber
  public void onObjectSubscriber (ObjectEvent event) {
    // can `somethingThatShouldNotBeNull` be null at this point or does the event bus know to wait for the object to be fully initialized?
  }
}

Thanks!

[Benjamin Manes]

You're describing a classic problem, described in the JSR-133 FAQ and in Goetz's Safe construction techniques article. You should register the listener after the object's constructor has completed, at which point the memory model guarantees it can be safely visible, instead of within the constructor.

--
guava-...@googlegroups.com
Project site: https://github.com/google/guava
This group: http://groups.google.com/group/guava-discuss
 
This list is for general discussion.
To report an issue: https://github.com/google/guava/issues/new
To get help: http://stackoverflow.com/questions/ask?tags=guava
---
You received this message because you are subscribed to the Google Groups "guava-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to guava-discus...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/guava-discuss/81c2dfcc-e4f3-4e4f-b78a-e960fdf97b9d%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[fid...@gmail.com]

Thanks for the quick response.

Is there a good pattern you can use to ensure that an object will always be registered on the event bus without having to ensure that every use of the constructor is immediately followed by a call to eventBus.regsiter() or a similar method.

Also, it may be good if it was documented somewhere in the event bus that this pattern could shoot you in the foot. We had no idea when starting our project.

Perhaps there should be a note on the register method? http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/eventbus/EventBus.html#register(java.lang.Object)

Thanks!

[Benjamin Manes]

It depends on your architecture. I've always used dependency injection to attach subscribers, such as Guice's multibinder or Spring's collector injection. In that case construction is safe, a little extra logic is placed in your module wiring, and its overall simpler as the code base grows.

To view this discussion on the web visit https://groups.google.com/d/msgid/guava-discuss/0b0ab116-8f22-46a3-aa4f-8f0905750793%40googlegroups.com.

[Luke Sandberg]

Without using DI like what Ben said.  The classic pattern is to make your constructors private and then uses static factory methods to attach listeners.  e.g. instead of

  public Test(EventBus eventBus){

    eventBus.register(this);

    // other stuff happens

    this.somethingThatShouldNotBeNull = new Object();

  }

  

You would do:

 public Test createTest(EventBus eventBus) {

    Test test = new Test();

     eventBus.register(test);

    return test;

  }

private Test(){

    // other stuff happens

    this.somethingThatShouldNotBeNull = new Object();

  }

  

This would ensure that there is no way to construct the object incorrectly.

To view this discussion on the web visit https://groups.google.com/d/msgid/guava-discuss/CAGu0%3DMPx0Oq%3D4wLENhBkWoDLZjLbR9LFs48XjD%2B80Z_VfPp9tQ%40mail.gmail.com.

[ctalsmag]

Often times when I use EventBus I use it in conjunction with Guava Services. More often than not my Services are the Objects which take action when Events are fired.

class TestService extends AbstractIdleService {
@Override
protected void startUp() {
myEventBus.register(this);
}

@Override
protected void shutDown() {
myEventBus.unregister(this);
}
....

You can start the Service with service.startAsync().

[fid...@gmail.com]

So you can have the same problem even if you register the object in the last line of the constructor of a final object because of how the java memory model works?
I'm just making sure that I understand.

-Jonathan Leitschuh

On Tuesday, November 17, 2015 at 6:36:51 PM UTC-5, Ben Manes wrote:

[Benjamin Manes]

Yes. JLS 17.5 and the paper Final Field Semantics provide good insights into this. Final fields are only frozen when the constructor is finished, and aggressive reordering within the constructor is allowed as long as very simple readability rules are followed.

To view this discussion on the web visit https://groups.google.com/d/msgid/guava-discuss/d6d5bf73-ed8c-4e43-b992-e1f41d9b0dce%40googlegroups.com.

[Chris Povirk]

Your original code can indeed fail, but I am going to disagree with some other parts of the answers.

It is true that final fields have special handling in the memory model. And it's true that, in some cases, code relies on that special handling. But such cases are rare, and they do not include code that calls eventBus.register(this).

This is guaranteed to work fine:

  public Test(EventBus eventBus) {

    this.somethingThatShouldNotBeNull = new Object();

    eventBus.register(this);

  }

It is true that your call to register(this) must come after your other setup. That's because, as soon as you call register(this), EventBus may invoke your handlers.

But as long as you've done your setup by that point, you're fine. There's no need for special constructor/final handling. In memory-model language, everything you've done before calling register(this) "happens before" register(this), and register(this) "happens before" any handlers are invoked. This guarantees that your constructor's work is visible to your handlers. (We should really document these happens-before relationships, but we've been lazy about doing so in general. But suffice it to say that, if they didn't exist, many EventBus users would not work at all.)

(But make sure that Test is final: If someone subclasses it, the entire Test constructor, including the register(this) call, will run before the subclass constructor has a chance to do anything. If for some reason you do need to subclass Test, you'll want to move your register() calls to a static factory method or Service, as described above.)

[fid...@gmail.com]

Chris,
The problem is that other threads may fire that event handler before eventBus.register() has returned. Since that memory is only guaranteed to be frozen and visible to other threads after the constructor is complete a subscriber could see variables that have not been frozen by the thread that registered the object.

Am I understanding this correctly? This is for a large application for my college senior project and we're getting a lot of race time conditions. I'm just trying to understand the problem fully.

Thank you for your help with this. I'm learning a lot.

-Jonathan

[Chris Povirk]

On Fri, Dec 4, 2015 at 4:02 PM, <fid...@gmail.com> wrote:

Chris,
The problem is that other threads may fire that event handler before eventBus.register() has returned.

It is true that events may fire before register() returns. But that's OK as long as you've done your work before calling register().

Since that memory is only guaranteed to be frozen and visible to other threads after the constructor is complete a subscriber could see variables that have not been frozen by the thread that registered the object.

There are multiple ways to guaranteed that values are visible to other threads:

• Store those values in an object's final fields, and make that object available to other threads without any synchronization actions. If you do this, there is no guarantee that the other threads will ever see your object at all. But if they do, you are guaranteed that they will see the values you wrote to the final fields. (There's slightly more to it than this, but that's the basic idea.) This is what is meant by "frozen."
• Store the values however you want, and make that object available to other threads through synchronization actions. Your object is guaranteed to be visible, and so are its values. EventBus performs these synchronization actions for you. The fields aren't "frozen" according to the definition above, but they are definitely visible to your handlers.

The conversation here has focused on the much more interesting first bullet. But it's the second bullet that is the much more common case. And it's only the second one that can guarantee that your register() call actually enables any calls to your handlers at all.

Now there's nothing wrong with moving the register() call outside your constructor. It's even possible that, when race conditions being what they are, it may eliminate the problems you're seeing (or make them worse, or who knows what :)). Maybe the move will compensate for other unsychronized operations elsewhere in the program. But I don't think you'll be addressing the underlying problem.