Dan Haywood created ISIS-948:
--------------------------------
Summary: 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)
