[
https://issues.apache.org/jira/browse/ISIS-948?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14208999#comment-14208999
]
ASF subversion and git services commented on ISIS-948:
------------------------------------------------------
Commit f90a62ec16986937ca12b21f39d1e66d7c344c1f in isis's branch
refs/heads/master from [~danhaywood]
[ https://git-wip-us.apache.org/repos/asf?p=isis.git;h=f90a62e ]
ISIS-948, ISIS-947: refactoring event bus, removing special case handling of
event bus in Isis core runtime; adding in ability to inject into request-scoped
services and for request-scoped services to be event bus subscribers.
Some of the new reference counting logic has moved from EventBusDefault into
EventBusService (abstract class in applib).
Also (for ISIS-947) includes enhancements to ServiceInstantiator so that can
call toString(), equals() and hashCode() even if there is no session.
In addition:
- ToDoItemSubscriptions (for todoapp) moved to menu item, rather than
contributed (better intent traded against slightly worse workflow during demos).
> Fix concurrent exception in EventBus, improve support for request-scoped
> services
> ---------------------------------------------------------------------------------
>
> Key: ISIS-948
> URL: https://issues.apache.org/jira/browse/ISIS-948
> Project: Isis
> Issue Type: Improvement
> Components: Core
> Affects Versions: core-1.7.0
> Reporter: Dan Haywood
> Assignee: Dan Haywood
> Fix For: core-1.8.0
>
>
> This ticket was prompted originally by [1], flagging up a concurrency issue.
> The background here is that EventBusService (a singleton) manages the
> registered subscribers for the guava EventBus, but the EventBus itself is
> stored internally by the framework on IsisSession, ie is in effect
> request-scoped. There are some special-case hooks to call open() and close()
> on EventBusService when the xactn is started/completed so that the service
> can call register() on the guava EventBus object for each interaction.
> The concurrency exception is (I think) because the set used in
> EventBusService was not thread-safe; should've been ConcurrentHashSet or
> similar.
> ~~~
> Thinking about this issue has led me to consider whether the guava EventBus
> should have been singleton rather than request-scoped, in particular with
> respect to whether we can accommodate request-scoped subscriber services as
> well as singleton services. Have come to the conclusion that it doesn't
> actually matter either way; it would be safe to have a singleton event bus
> having request-scoped services as subscribers because the proxies for those
> services would always route the method call to the on(Event) to the
> appropriate underlying service from its proxy's threadlocal.
> That said, have decided to keep the EventBus request-scoped. But rather than
> have all the special case logic, instead wrap the guava EventBus in a new
> RequestScopedEventBus domain service (as its name suggests, is
> @RequestScoped) and have this act as the container of the guava EventBus and
> also keep track of the subscribers.
> For its part, the original EventBusService remains as a singleton whose job
> it is to keep track of all the subscribers. Some of these are singletons
> that will register in the system bootstrapping (during their @PostConstruct
> lifecycle). Some of these might also be request-scoped services. So far as
> possible want the model to be the same... that they can register in their own
> @PostConstruct lifecycle. Now for these request-scoped services, the
> @PostConstruct should actually be called after injection of the
> EventBusService... The one slight modification is that they should not
> register themselves but instead their proxy. They can get hold of their
> proxy simply by having it be injected:
> eg.
> {code:java}
> @RequestScoped @DomainService
> public class MySubscribingService {
> private EventBusService ebs;
> public void injectEventBusService(EventBusService ebs) { this.ebs = ebs; }
> private MySubscribingService proxy;
> public void injectProxy(MySubscribingService proxy) { this.proxy = proxy;
> }
> @PostConstruct
> public void startRequest() {
> ebs.register(proxy);
> }
> @PreDestroy
> public void endRequest() {
> ebs.unregister(proxy);
> }
> }
> {code}
> Note also that we now allow injection into request-scoped services (though
> must use injectXxx rather than @Inject xxx).
> In the EventBusService, because request-scoped services might be continually
> registering and unregistering, but also each will be registering their proxy,
> then this needs to change to do reference counting. Thus, if there are two
> concurrent threads then the MySubscribingService (above) will be instantiated
> twice, each held in the thread-local of the proxy. This proxy will (in the
> @PostConstruct) be registered with the EventBusService twice (for each
> thread), but of course that proxy should only be registered with the guava
> EventBus once.
> The RequestScopedEventBus is responsible for instantiating the guava
> EventBus, but it does this lazily, when the first call to post(...) is made.
> At this time it calls back to the EventBusService asking for the current
> subscribers. The EventBusService will return all the singletons (because
> they will always have a reference count = 1) plus it will return the proxy of
> all the subscribing request-scoped services (because there will always at
> least one current thread that would've caused its proxy to have been
> registered).
> As I say, an alternative design could probably have dispensed with the
> reference counting and having the guava EventBus being request-scoped; this
> simpler design would instead simply hold a set of subscribers in
> EventBusService. The singleton subscribing services would all be added
> during bootstrap, while the (singleton) proxy for all the request-scoped
> subscribing services would be added in the very first transaction.
> Might end up refactoring to this simpler design as and when we bring CDI into
> the mix.
> [1] http://isis.markmail.org/thread/o5zbm54cpb4tizqt
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)