Service.exception uses static ExceptionMapper & Thread Safety issues on stop

[dbavery]

I have found a couple of issues with using Service.ignite() mostly around concurrency and stale information. These were discovered in our unit test environment when we were trying to fire up multiple Service instances.

The first problem is that the exception mappers are statically defined, so the exception mappers from one instance of the service can (and do) conflict with other instances of services. This is mostly a problem in unit test set ups where the static exception mappers have to co-exist with two Services that could have different behaviors for the same exception. The best example here is define two different mappings for Exception.class

Second issue is that when stop() is called, while stop() is a synchronized method, the actions take are within a new thread and those actions are not synchronized on the same threads. So an action, such as clearing out the exception mapper (and seeing initialized to false), happens AFTER you have left the synchronized block, leading bad memory references.

At first glance the easy fix for this(these) issue(s) are the following:

1) Use an instance fo ExceptionMapper instead of the global getInstance() -- https://github.com/perwendel/spark/blob/master/src/main/java/spark/Service.java#L87 2) In your "stop" thread, make sure that you synchronize on the same object (i.e. Service.this) inside your stop thread, OR make sure that the objects being used are properly protected with volatile/synchronized/locks. - https://github.com/perwendel/spark/blob/master/src/main/java/spark/Service.java#L464

[skyghis]

Seem to kind of duplicate issue #570.

Would be great to see this merged.

[perwendel]

Static exception mapper has been solved by #1033

[jeraymond]

The static exception mapper change seems to have caused #1062 which breaks exception mappings for standalone web servers such as Tomcat. With each service getting its own ExceptionMapper the SparkFilter no longer references the right mapper instance when registering exception handlers via the static exception() method.

See #1062 for a workaround.