JSR-315: JCP Failings
JSR-315 has produced a Public Review (PR) of the servlet 3.0 specification, which unfortunately is a poor document and the product of a discordant expert group (EG) working within a flawed process. I know this article will be read as a criticism of my fellow EG
members, however I genuinely feel that the failings are mostly those of the
process that we have been put in, combined with a touch of arrogance on
all our parts to believe that strength of our experience and opinions alone can overcome
the deficiencies of the process. However, whatever the deficiencies of process, JSR-315 has produced a draft specification that has some significant technical flaws, not the least with pluggability and asynchronous servlets. The EG is desperately in need of community feedback and probably another round of Public Review after a beta RI is made available.
Thought Experiments: No working prototypes.
Unlike the IETF RFC tradition of pragmatic, experience-driven, after-the-fact
standards authorship, the JCP as applied to JSR-315 has no such tradition nor is a reference implementation required by the JCP until after the public review is completed and just before sending the
specification to the responsible EC for final approval. This public draft of the servlet-3.0 represents a thought experiment in API design unhindered by the complexities of implementation, trial usage or community feedback. As a member of this Expert Group, I’ve been as guilty as the other members by looking at uncompiled/untested APIs and believing them to solve the issues at hand, only to later discover that these APIs prove to be unworkable when implemented and/or used. It is entirely unacceptable that such a significant revision of one of the corner stones of Java Application development is being put forward almost completely untried and untested. Requests for test implementations have been denied and my own partial implementation in a jetty branch, has revealed significant unresolved problems.
Consultation: these are not the droids…
There is no open or considered mechanism to collect requirements from the community and precious little community consultation. The deliberations of the EG are not open and it is difficult to obtain community feedback. Any feedback received on the jsr-315-comments@jcp.org list have been kept private and not shared even with the EG, despite several requests. Without any true representation of community concerns, it is too easy for concerns to be discounted. For example, several EG members and community feedback raised concerns about the security and ordering aspects of the automatic deployment possible with the Annotations and Pluggability feature (see below). The response was not the “How can we address these concerns” discussion that I was expecting. Instead the response was more along the lines of Obe-wan Kenobi saying “these are not the concerns you are looking for. Move along!”
Alternately, some unsubstantiated requirements (eg. wrapped asynchronous requests) have been included at a late stage without any use-cases or user demand, which have introduced whole new dimensions of complexity so that discussions have been ratholed for months chasing marginal edge cases.
Precision: ambiguous language
When one reads a IETF JSR the language used is precise and codified, and they are full of state machines and interaction diagrams. This is not the style of most JCP produced specifications, which are instead full of prose that frequently looks like it has been written by engineers for whom a two line comment is a challenge (myself included). Within the servlet EG, our efforts to formulate the future of the specification are often hampered by our inability to fully comprehend what we wrote last time around. This lack of precision works to prevent true consensus being formed, as participants often think that an approach has been agreed, only to later discover that that multiple conflicting interpretations of the proposals exist and that issues thought to be resolved are not.
JCP or JSR Failings?
I am only a member of the JSR-315 EG, so I do not know if these failings are truly general to the JCP or just specific problems for the Servlet JSR. However, whatever their generality, these failings have resulted in a seriously compromised Public Draft specification and I suspect the problems I describe below are only the tip of the iceberg and that more issues will be revealed by real testing.
Annotations and pluggability
The ED of JSR-315 introduced three new mechanisms designed to make it simpler to plugin web frameworks to a web application so that placing a JAR file into WEB-INF lib is all that is required to instantiate a web framework and to initialize a default configuration:
- Additional annotations
have been defined so as to be able to configure filters and servlets entirely
from annotations of classes contained within WEB-INF/lib or
WEB-INF/classes
- Support for web.xml
fragments to be included in the /META-INF directory of jar files within
WEB-INF/lib. These web fragments are combined with the web.xml with
well defined merging rules (already mostly defined when arbitrary
element ordering was supported in 2.5 web.xml)
- Programmatic
configuration of Filters and Servlets via new methods on
ServletContext. These methods are only active when called from a
Listener class defined either in a web.xml fragment or discovered TLD
file (but can execute arbitrary code as well as calling the new APIs).
These are good new features and could address some long standing pain points of web application development, not least the contention of editing multiple concerns into a single web.xml. Unfortunately the many concerns raised with the early draft have not be addressed and the EG members and community who have raised them have simply been told not to be concerned with issues such as:
- Accidental Deployment: Web applications can
contain many many third party jars and a annotation, web fragment or TLD listener in any them will result in arbitrary code being executed and the deployment of servlet and filters. These servlets and filters may well be harmless debugging aids or admin UIs that the framework developers thought useful or they may be hacker attacks buried deep inside jars imported as transitive dependencies. The automagic nature of the pluggability features now requires that deployers perform a whole new level of due diligence before deploying or upgrading any jars within a web application. Tools will not be able to
greatly assist with this process as the analysis of the programmatic
configuration of listeners is a NP-complete problem and never again will we be 100% sure of what servlets can be deployed unless we 100% understand all the source code of all the jars within a web application!
These features do not give attackers any new ways of injecting hostile code into a web application, however they do provide new mechanisms to execute any code they manage to inject and that code will be able to easily open access through corporate firewalls or filter all web traffic!
The only solution available to those concerned about accidental deployment is to turn off all these features with meta-data complete and get no benefit from them at all.
- Slow Deployment: Web applications can
contain many many third party jars. Already with TLD scanning, web containers need horrible NO TLD JARs configuration to reduce the time to deploy a web application to something reasonable. Now with scans of every class for annotations, such hacks will become even more important.
- Ordering:
The ordering of auto discovered configuration was not defined in the ED. Those that raised the concern were told that their proposed solution was too complex and that a solution was in the works. After months of waiting, the solution was finally revealed with this text from the PD: “The order in which configuration settings of web fragments are added to those in the main web.xml is undefined.”! Great! So if you wish to plug in a security framework (eg acegi) with a Ajax framework (eg DWR), then you cannot specify order unless you disable pluggability and copy all the configuration into a single web.xml
- Parameterization: WAR files have often been criticized for having configuration baked into the jar file, with a frequent complaint being that a war file needs to be unzipped, simply to change a few parameters within the web.xml and then zipped up again. Apart from being a PITA, this need to edit a deployment unit is bad practice! These features exacerbate this issue, as configuration can now be baked into a jar file that itself is baked into a WAR file. If the default configuration is unacceptable, then a deployer can either disable the features and copy everything into a web.xml (assuming they know what “everything” is), or they can unzip the war to unzip the jar to edit the webfragments contained within. Of course such editing will blow away any attempts to use checksums or signatures to check for any changes that might result in accidental deployments!
When raising these concerns, several EG members proposed several solutions based on giving the web application the optional ability to participate via a standard configuration in the selection of which jars to scan and/or ignore and to possibly pass parameters to those jars. Such proposals would address all the concerns listed above and such mechanisms ALREADY exist in most containers, yet it was decided it
was too complicated to expose users to such choice, and far better to
keep such configuration buried in static code or non-standard
configuration.
It is not sufficient to respond to such concerns by saying we need not be concerned. I am concerned, members of the jetty community are concerned, obviously those that implemented noTldJars in Tomcat and Glassfish are concerned, many commenters on The Server Side are concerned. Yet all proposals to address these concerns were dismissed simply as not needed and/or too complex. No counter proposals were forthcoming, so these potentially useful features will be disabled by a large proportion of users and should be disabled by many more.
Note that these are only the issues discovered by reading the specification.
I’m sure if somebody actually implements these features and tries to
test them, additional issues will be discovered. Unfortunately, the one
framework developer on the EG was denied when he offered to test these
features before their inclusion in the spec, since there is no available implementation.
Asychronous Servlets
Early Draft Version
The early draft of JSR-315 Servlet 3.0 specification contained a proposal for asynchronous servlets that was developed based on my experience with Jetty Continuations and has been available as a trial implementation in the pre-releases of Jetty-7 since March 2008 which has been tested by many frameworks and applications, including cometd, DWR, JSF and BlazeDS. The proposal involved new methods on the ServletRequest class:
interface ServletRequest{ void suspend(long timeoutMs); void suspend(); void resume(); void complete(); boolean isSuspended(); boolean isResumed(); boolean isTimeout(); boolean isInitial(); ...}
As well as the specification and javadoc, I’ve demonstrated the usage of these methods in Jetty-7 and in my blogs on asynchronous use-cases and patterns. As far as I know (and I have asked), no negative feedback at all was received on the jsr-315-comments@jcp.org, while I have seen some very positive public feedback (eg DWR, JSF and BlazeDS). While the proposal could certainly have been improved (see below), it was a very encouraging start.
However, after Javaone, a campaign was begun for an alternative asynchronous proposal. While I welcome alternatives, this one was based on criticisms of the ED proposal that were factually incorrect (eg. that all requests would need to be wrapped or that redispatching is new and unnecessary behaviour). Even though these criticisms were completely rebutted (not least by Sun’s own JSF project providing a lifecycle to use suspend/resume without the need for any framework changes) and later retracted, they were used as an basis of an effective veto of the ED proposal, based on it’s use of redispatch semantics.
I would have expected to have
been given the opportunity to refine the ED proposal, but for a period of time any proposal that included redispatch semantics was completely rejected. This veto resulted in the bizarre positions that asynchronous servlets would have been able to generate a response using any technique they liked EXCEPT by dispatching to a servlet engine. Of course this argument could not be sustained and eventually redispatch semantics has come back into the all active proposals, but only after the ED proposal had been killed off.
So rather than building on the tried and tested ED proposal, the proposal in the PR proposal represent a snapshot of one of the alternative proposals that have been developed as thought experiments without any implementations or test code. In my desire to find consensus, I have been supportive of some of the alternative proposal and have participated in the thought experiments. Unfortunately I had fallen victim to the same hubris that I
criticize the EG of, and subsequent attempts to implement these proposal have revealed significant problems.
Public Review Version
The proposal in the Public Review grew out of a failed attempt to use RequestDispatchers as the asynchronous redispatch mechanism. Significant effort has been put into the PR proposal to try to make it handle all concerns raised but unfortunately it has become a Frankenstein monster, cobbled together from eviscerated good ideas and misguided best intentions. The 8 methods of the ED proposal that were rejected as being too
complex and confusing have been replaced by 20 methods and 3 new
interfaces!
The PR proposal is based on a AsyncContext class that is obtained from a request by calling a startAsync method together with a slightly torturous timeout mechanism:
interface ServletRequest{ boolean isAsyncSupported(); AsyncContext startAsync(); AsyncContext startAsync(ServletRequest servletRequest, ServletResponse servletResponse); boolean isAsyncStarted();
AsyncContext getAsyncContext(); void addAsyncListener(AsyncListener listener); void addAsyncListener(AsyncListener listener,ServletRequest servletRequest,ServletResponse servletResponse); void setAsyncTimeout(long timeout); ...}
interface AsyncContext{ ServletRequest getRequest(); ServletResponse getResponse(); boolean hasOriginalRequestAndResponse(); void forward(); void forward(String path); void forward(ServletContext context, String path); void complete(); void start(Runnable run);}
interface AsyncListener{ void onComplete(AsyncEvent event) throws IOException; void onTimeout(AsyncEvent event) throws IOException;}
class AsyncEvent{ ServletRequest getRequest(); ServletResponse getResponse();}
The key features that this proposal is based on are:
- that asynchronous filters/servlets should have the option to preserve the request/response wrappers applied to a request for use either by asynchronous handlers or by a redispatched request.
- that redispatch asynchronous requests should use RequestDispatcher forward semantics and may be dispatched to different contexts and/or paths than the original request.
These features were added as requirements without any supporting use-cases and without any user demand. At best they can be looked at as nice to haves. Yet they both introduce incredible complexity that we have only just started to fathom. The result has been that discussions have been ratholed for months chasing these unjustified edge cases.
Why keep wrappers?
One of the prime motivations of asynchronous programming – indeed THE prime motivation, is to avoid holding resources when you are not doing anything! So why keep wrappers during asynchronous handling of a HTTP request? They will only be holding resources (buffers, Gzip encoders, JDBC connections, UserTransactions etc.) that would be better off returned to a pool or GC. If the redispatched request needs those wrappers, then reapply them during redispatch. If it is too expensive to wrap twice, then suspend the request before you wrap them the first time.
The cost to support asynchronous wrappers is extreme, as it costs us the sanctity of the try{} finally {}! Typically filters that apply wrappers can clean up those wrappers in finally block, knowing with 100% certainty that their code will be called when the wrapper is no longer applied. The general pattern of such a filter is:
class MyFilter implements Filter{ public void doFilter(ServletRequest req,ServletResponse res,FilterChain chain) { MyResponseWrapper wres= new MyResponseWrapper(res); try { chain.doFilter(req,wres); } finally { wres.finish(); }}
But with the PR proposal, this common pattern is no longer sufficient. A wrapper can be passed to an asynchronous handler in a startAsync(req,res) call and if so, it should not be “finish”ed in the finally block. Thus all well written filters that wrap request or responses will have to allow for this by code that looks like:
class MyFilter implements Filter{ public void doFilter(ServletRequest req,ServletResponse res,FilterChain chain) { MyResponseWrapper wres= new MyResponseWrapper(res); try { chain.doFilter(req,wres); } finally { if (request.isAsyncStarted() && !req.getAsyncDispatcher().hasOriginalRequestAndResponse() { req.addListener(new MyFilterAsyncListener() { public void onComplete(AsyncEvent ev) { ((MyResponseWrapper)ev.getServletResponse()).finish(); } },req,wres); } else wres.finish(); } }}
This is for code that does not even use asynchronous features! It must be implemented by a filter simply so it can work with other filters/servlets that might use asynchronous features and that might want to keep wrappers for some not-as-yet identified use-case!!! The advocates of wrapping asynchronous requests talk about starting producing a respons synchronously in a servlet and then passing control to an asynchronous handler and preserving the state of the wrappers – but they cannot answer the question why one would want to program in such a way or why the servlet spec should support it?
The sanctity of try{} finally {} should not be broken for such a marginal use-case and wrappers should only be valid within the scope of the filters/servlets they are dispatched to. If the cost is that wrappers cannot be held between async dispatches, then it is a small price to pay keep an important programming paradigm intact!
Why forward?
During the development of the PR proposal, at one stage normal RequestDispatchers were proposed as the asynchronous redispatch mechanism. This initially attractive idea falls down because the threading model is wrong and dispatchers are designed with the assumption that they are called from an already dispatched filter or servlet. Unfortunately, which the use of RequestDispatchers was dropped, the desire to forward asynchronous requests to another path or another context was not and forward semantics has been adopted as the semantics of asynchronous redispatch.
Why one would want to forward to another path is beyond me, and forwarding to another context from an asynchronous handler is verging on scary. But if there was a use-case for such a thing (and none has been identified), it would have been trivial with the ED suspend/resume proposal to resume the request and then use the normal RequestDispatcher mechanism to direct the request to the desired handler (there appear to be no efficiency gains from forwarding and then resuming rather than resuming and then forwarding!).
But the problem is that even if forward() is used and the path is not changed, forward semantics are used for the redispatch. Apart from the wasteful setting of 5 javax.servlet.forward.* attributes, this semantic means that asynchronous dispatches bypass filters that are not marked for FORWARD dispatches. If in response, you then configure your filters so that they do apply to FORWARD dispatches, then there is no way to tell if a request is being forwarded normally or has been asynchronously forwarded. For example, if you want a GzipFilter to apply to asynchronous dispatches, it is very difficult to avoid double wrapping normal forwards. If it was thought that redispatching as a normal request was too confusing for developers, I cannot see how mixing asynchronous dispatches with normal forwards is any less confusing!
Allowing asynchronous forwards is a feature in search of a use-case. There is no identified need for it and it is the cause of considerable confusion and complexity in the PR proposal.
Only Simple examples work!
With these issues, it is possible to create a few simple examples that work well enough stand alone. However, as soon as you try to combine these examples with other asynchronous concerns (eg QoS) or even simple GzipFilters, these issues soon become apparent and I’ve been unable to reimplement most of the async fuctionality demonstrated against the ED.
Improving the Early Draft proposal.
While I believe the ED proposal was fundamentally sound, the EG did indeed highlight some issues and the alternative proposals do include good ideas. Thus I’d like to examine what the API could have looked like if we had applied the expertise of the EG towards refining the tried and tested EG solution. The issues with the ED that were identified include:
- Bad names. While I don’t particularly like “startAsync”, I do concede that “suspend” and “resume” are overloaded terms and could imply behaviour that is not supported.
- Dispatch Type. While I’m not 100% convinced and many frameworks
have coped, I can see the argument that mixing resume dispatches with
normal request dispatches could be confusing and isInitial() was an
in-elegant way to deal with separation of those mixed concerns. A
better approach would be to create an entirely new dispatch type for
async and timeout dispatches with a method on the request to query the dispatch
type (which would be generally useful anyway).
- Resume Race. A common programming bug with suspend/resume is to write code that looks like:
if (shouldSuspend()){ MyAsyncHandler ah = new MyAsyncHandler(request); request.suspend(); return;}
but this code contains a race, as the MyAsyncHandler can attempt to resume the request before suspend is called. The fix is to call suspend before creating the async handler, but that can read a little unnaturally. Having the resume method on an object returned by the suspend avoids the possibility of this race.
- Difficult Debug. Code that was developed before asynchronous servlets could fail in hard to diagnose ways if combined with filters/servlets that are using asynchronous techniques. The PR proposal contains isAsyncSupported mechanism to allow servlets/filters to declare that they have been tested with asynchronous features and to generate meaningful errors if code is unknowingly exposed to asynchronous requests.
- Wrapped Requests. Asynchronous handlers could be passed references wrapped requests that would be destroyed in their filters finally blocks. The use of AsyncContext bypasses such wrappers.
These issue can be fixed in ED proposal without throwing out the basic principal: that the servlet spec well defines behaviour within a dispatch to a filter/servlet and that rather than invent a new contexts for application code to execute in, it is far better to dispatch back to the well define servlet container in the most simple manner possible and to thus build on a decade of experience with filters and servlets. The evolved ED proposal looks remarkably like the PR proposal, but without the distraction of wrappers and forwards.
interface ServletRequest{ DispatcherType getDispatcherType(); boolean isAsyncSupported(); AsyncRequest startAsync(long timeoutMs); AsyncRequest startAsync(); boolean isAsyncStarted(); AsyncContext getAsyncContext(); ...}
interface AsyncContext{ ServletRequest getRequest(); ServletResponse getResponse(); void dispatch(); void complete();}
public enum DispatcherType { REQUEST, ASYNC, TIMEOUT, FORWARD, INCLUDE, ERROR}
Note the DispatcherType enum is already part of the PR specification for other reasons, and only the ASYNC and TIMEOUT types have been added to it.
This evolution of the ED proposal builds on all the testing and experience gained, while fixing some important issues. It does not support wrappers or forwards, but nobody has been able to cite a use case that justified either. If use-cases were identified, then I’m confident that the ED API could be further revised to meet those use-cases without the need for wrappers or forwards. Surely if wrappers and forwards features prove necessary, they could be introduced in a 3.1 servlet spec after we have gained experience with the core asynchronous principals?
I plan to have an implementation of this revised ED proposal available in a branch of Jetty within a few days and will be proposing that this be the basis of a second PR of the specification.
Conclusion
These are important matters that will have long lasting effects. The API decided by JSR-315 will be with us for many years to come and will affect the evolution of java web applications and potentially the rest of our programming careers. It’s not something that we want to get wrong, or even just approximately right.
I believe significant errors are being made in the current PR and that the flaws in the process have been enough to prevent the application of due diligence sufficiently for these flaws to become self evident. While I have received support in the EG with the debate on these issues, I have been unable to convince the spec lead of their validity and I have probably not helped the cause by some over-vigorous participation in the debate.
However, the JCP is the Java COMMUNITY Process. So where I have failed to make the case, the community now has an opportunity to make it for me (or indeed to correct me in the error of my ways). If you are reading this, then you are part of the community! If you ever wish to complain about these APIs, then now is your chance to contribute to the process (or at least take the I-told-you-so moral high ground):
The EG is desperately in need of community feedback and probably
another round of Public Review after a beta RI is made available.