We've had a report of the following behaviour from JBoss Resteasy:
- servlet calls `request.startAsync()` and then starts 2 non container threads before returning.
- Thread-A blocks in a call to `request.getInputStream().read()`
- Thread-B calls `asyncContext.complete()`
Because there is no container thread dispatched to the app, the handling of `complete()` can start straight away. But what should be done with the blocked read?
Is this RESTEasy doing this, or a user app written against RESTEasy? AFAIK RESTEasy won't start its own threads after suspending, so it sounds like this might be a user app that has created the multiple threads?
Perhaps the blocked read should throw an IOexception, but then the app might catch it and start operating on the request/response, which may be recycled to have been reused.
In general you should unblock the thread, because blocking a thread indefinitely is not nice.
So is it an error to call `complete()` in this way? The spec says that request/response are not thread safe in general, so is having 2 threads operating on them in this way supported? Shouldn't the app be required to make sure other threads have stopped operating on the request/response before doing a complete? So complete could throw an ISE in this circumstance.
This case sounds like it's the applications fault, but consider an app that calls start async, then does a blocking read or write from an executor. If this takes more than 30s then then request will timeout, and the app server will dispatch a thread to call complete. We now have basically this same situation in the most simple of use cases (also a blocking read is not even the main issue here, more that you might end up with two threads attempting to write a response).
This is basically inherently racey as it stands no matter what we do. Consider the following case:
- App starts a thread and does a blocking read, read takes exactly the same amount of time as the timeout
- We time out and a users timeout listener begins to write an error response of some kind (setting headers, status code, writing content etc)
- Actual app thread has finished reading and writes a response at the same time, setting different headers, status code and content.
You could end up with a situation where the error response is mixed in with the actual app response, wrong content lengths and all kinds of problems.
I can only really think of one way to fix this, and that would be to add a special wrapper HttpServletResponse around the actual response for async servlets. This wrapper
would have the ability to atomically dissociate from the underlying response to basically stop the application thread from making any more changes. Once this disassociation happens then
the thread that is doing the complete() has basically taken ownership. This has issues from a spec perspective though, as the wrapper may have been wrapped again by filters, so the timeout listener
may be expecting to be writing to the wrapped response (which won't work, because they are now wrapping a response that is no longer associated with the underlying connection).
Note that this wrapper approach will likely require a fully syncronised response in order to be 100% safe, as when you disassociate you need to be sure that threads are not currently active in the underlying response.
I think the only reason that we have not seen bigger issues around this is that the chances of a users operation finishing at exactly the same time as the timeout is super low, so even though this is fundamentally dangerous
in practice it is very rare.
As it stands I don't think we can fix this with a simple clarification, as IMHO there are fundamental issues here.
Stuart
_______________________________________________
servlet-dev mailing list
servlet-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/servlet-dev
