Subscriber defined threading.

[jhugman]

In many event buses, the event poster is responsible for deciding what thread the event will be delivered on. This often leads to confusion and subtle threading bugs. This confusion may be duplicated on every event post.

This patch adds an optional thread parameter to the Subscribe annotation. This lets the subscriber define what thread it needs to run in.

@Subscribe(thread=ExecuteOn.BACKGROUND) updateDatabase(UpdateEvent event);

@Subscribe(thread=ExecuteOn.MAIN) updateUi(UpdateEvent event);

@Subscribe(thread=ExecuteOn.ASYNC) updateRemoteServer(UpdateEvent event);

void update() {
   bus.post(generateUpdateEvent());
}

It is backward compatible with noarg @Subscribe annotations. The annotations on the following two method signatures have identical semantics.

@Subscribe updateInAnyThread(UpdateEvent1 event);

@Subscribe(thread=POSTER_DECIDES) updateInAnyThread(UpdateEvent2 event);

This is a strawman implementation that defines four possible choices of threading that a subscriber can run on:

• MAIN - the UI thread,
• BACKGROUND - a single background thread,
• ASYNC - a thread from an unbounded thread pool and
• POSTER_DECIDES - the current behavior. This is the default.

For best effect use ThreadEnforcer.ANY, so events can be posted from non-ui threads.

bus = new Bus(ThreadEnforcer.ANY);

This should solve previously closed issues #14 and #38 reasonably cleanly.

[JakeWharton]

Hi James,

First of all, thanks for using and working on Otto. I'll tell you up front that as this pull request stands we won't be accepting it. Let me outline a few reasons why below...

This often leads to confusion and subtle threading bugs.

The thread confinement as it exists now eliminates this. We intentionally enforce the Android main thread by default because that is what you should be using 99.9% of the time. This will actually keep your code clean because you always know on which thread the interaction will occur.

As you mentioned, you can remove the thread confinement with the ANY strategy. This was intended for three places:

1. Testing on the JVM
2. Applications on the JVM
3. Extremely rare cases where background threads need communication that never touches the UI.

Back to your implementation, you say that by making the poster determine threading it can lead to subtle bugs. Your implementation suffers the same problem but inverted. Here's an example:

@Subscribe(MAIN) updateUi(UpdateEvent event) { /* do something */ }
@Subscribe(BG) updateFilesystem(UpdateEvent event) { /* do something */ }

List<Users> users = userService.list();
UpdateEvent event = new UpdateEvent(users);
bus.post(users);
Collections.sort(users);

In this example, my UI thread will get the ordering from the remote server whereas the filesystem callback gets a list sorted differently.

These types of things are what Otto was designed to eliminate.

The asynchronous nature of parts of this implementation also will now hold references to any objects posted for an unknown amount of time until the main thread dispatch can occur. If the object falls out of scope quickly, this prevents garbage collection.

We advertise Otto as a replacement for the traditional manger/listener model where you register a listener who invokes methods on you when "interesting" things happen. These simple method calls very obviously happen on whatever thread the manager calls them on. We want Otto to behave this way so that the threading decisions and movements are on you, the very same as it would be if you were doing this manually.

Additionally, as a user I think this API is too complicated which will likely lead to threading bugs due to lack of understanding of what each annotation option entails. The extreme simplicity of Otto's API and behavior is actually one of it's core features. It's intentionally a relatively "dumb" implementation so that you get zero unexpected behavior when using it.

Now, what you have here isn't all bad despite everything I wrote. Before this I was already convinced that we needed a thread-moving bus implementation standardized. You can very easily write an extension of the bus which synchronously moves posts to an appropriate thread in about 20 lines of code. We'll likely include one in the next major version of the library.

I encourage you to stay tuned for Otto 2.0's feature set. I think you'll find that it meets most of your requirements while still remaining extremely simple yet explicit.

[jhugman]

Hi Jake,

Thanks for looking at my pull request.

I realize that my implicit assumptions were that events were immutable, and that mutating events after dispatch would (certainly for me) feel a bit icky. Given enforcing immutability of an arbitrary object passed to you would be difficult at either compile time or runtime, this implementation of thread shifting would rely on those similar aesthetics. This convention is also expected by Scala Actors.

I understand that one of the design tenets of Otto is extreme simplicity, and thread moving is not part of it. (I am resigned to the fact that you're not going to take my patch :().

However, developers are already trying to deliver messages to subscribers on different threads. We are encouraged to move functionality off the UI thread as much as possible.

Relying on the caller/poster to post on the correct thread leads to a whole bunch of problems (for me):

1. The poster needs to know what is listening and what thread the listeners want to act on. This is an encapsulation problem.
2. Every poster of the same event all need to get the correct thread. This starts off with lots of boiler plate Runnables and Handlers all over the place, and then tends to be moved to utility methods on bus. Nevertheless, the developer needs to think about threads each time she posts a message, and again when she wants to refactor a subscriber to run on a different thread.
3. Subscribers to the same event cannot be on a different threads. My subscribers updateUi and updateRemoteServer were a deliberate examples of where this is not desirable. I cannot think of a way to achieve this with a single bus, a proliferation of Message classes or each subscriber manually looking after Thread shifting.

Moving the threading decision to the subscriber is the important bit here. Putting an optional parameter on the annotation seemed like the least disgusting way of achieving this.

If you are looking at thread moving for version 2.0, and I have any say in it, then I would very much prefer a subscriber/actor chooses. Let me know if I can do anything to help.

[JakeWharton]

We are undergoing discussions around this internally. It probably will end up like you wanted but with a bit less configuration and choice. Simplicity is an extremely high feature of Otto and we want to keep it that way.

Are you using your version of Otto with these features you've implemented? I'm curious as to whether you use all of the threading options that you provided.

[jhugman]

I understand (and value) the desire to keep Otto simple.

I'm currently using subscribers for:

• UI manipulation on the MAIN thread
• database reads, networking, disk I/O etc with ASYNC threads
• database writes, and a few non-pure functions on the single-threaded BACKGROUND thread.

I'm almost never using the default/POSTER_DECIDES thread.

I quite like the string identifier idea put forward in #74, as I can see that there may be times when there may be a need for running lots of subscribers in a single-threaded context, but some of them are parallelisable. e.g. have database writes go in a single thread, and have another background thread.

However:

• I prefer enums to magic strings.
• ASYNC threads are really useful.

If it helps you any with simplicity, other things I considered:

• using value() instead of thread() in the @Subscribe annotation. This would make subscription more terse e.g. @Subscribe(ExecuteOn.MAIN), though would mean you can't add any more parameters.
• having @ExecuteOn annotations, either with a single annotation with a parameter, or multiple annotations. I didn't think this was particularly discoverable as an API.
• using the ThreadEnforcer to choose the threads if thread parameter is missing. This would mean that the defaults change based on what you passed to the constructor of Bus. This sounds sensible, but I think would end up confusing.