I was under impression that leakage is implementation detail. We did not have to specify anything for sync case, why should we specify it for async case. There's nothing special about async case in this regard.
That said, I'm fine with adding a short sentence to the AR.resume()/cancel() javadoc stating that "A successful invocation of this method eventually completes the suspended request processing." or similar. FWIW, I do not want to write anything like "when this method completes, the request processing is complete", because I do not want to place restrictions on implementations. E.g. implementors may decide to complete the response processing asynchronously, which means that when the method returns the connection does not have to be necessarily closed yet. Which is also why I think it's better to not say anything...
Marek
On Feb 5, 2013, at 6:03 PM, Bill Burke <bburke_at_redhat.com> wrote:
> I don't agree this should be undefined by the specification...Not completing/closing the connection can result in leakage. The language in the javadoc/spec doesn't have to be "closed", but maybe "finished" or "completed". This would be the undefined part of the spec. What "finished" means to a container is up to the container.
>
> Leaving unmapped exceptions undefined is also a bad idea. While you can just propagate unmapped exceptions in the synchronous JAx-RS invocation, you can't propagate them here. SO, we need to define the behavior as we can't punt to the HTTP container (aka servlet container).
>
> This is my current pseudo logic (I posted this before):
>
> boolean resume(Object obj) {
> if (completed)return false;
>
> try {
> sendResponse(obj);
> connection.complete(); // whether this is a close or a servlet 3.0 complete, depends on container
> callbacks.onComplete(null);
> } catch (Throwable t) {
> return resume(t);
> }
> return true;
> }
>
> boolean resume(Throwable t) {
> if (completed)return false;
>
> try {
> if (mappable(t)) {
> mapException(t);
> } else {
> sendError(500);
> }
> } catch (Throwable t) {
> log.error(t);
> } finally {
> connection.complete(); // whether this is a close or a servlet 3.0 complete, depends on container
>
> }
> callbacks.onComplete(t);
> return true;
> }
>
>
> The Javadoc should change to say that the connection will be completed after resume() is finished. If there is an exception, it will be handled according to the resource method rules. Unhandled exceptions are turned into a 500, InternalServiceError response to the client.
>
> These methods return false if the connection has been closed or completed already.
>
> On 2/5/2013 11:47 AM, Marek Potociar wrote:
>> As for closing connections in case of an unmapped response, it should be IMO left up to the underlying container impl. Typically the container would map the exception to a HTTP 500 response, send the response back to the client and close the connection. If that's not possible due to an error while sending the response, the event should be properly logged IMO. But I would not want to put anything like that in the spec. We do not specify this case for sync processing and async is no different in this context.
>>
>> Marek
>>
>> On Feb 5, 2013, at 2:48 PM, Bill Burke <bburke_at_redhat.com> wrote:
>>
>>>
>>>
>>> On 2/5/2013 8:37 AM, Bill Burke wrote:
>>>> Javadoc says for AsyncResponse.resume(Throwable)
>>>>
>>>> * For the provided throwable same rules apply as for an exception
>>>> thrown
>>>> * by a {_at_link javax.ws.rs.HttpMethod JAX-RS resource method}.
>>>> * The processing of the throwable by JAX-RS framework follows the
>>>> same path
>>>> * as it would for any exception thrown by a JAX-RS resource method.
>>>>
>>>>
>>>> So, if the Throwable is a WebApplicationException, or there is a mapper,
>>>> then resume can send a response. If neither of those conditions are
>>>> met, then return false?
>>>>
>>>
>>> ANother question is, shouldn't the connection be closed if there is an error while resuming or cancelling?
>>> --
>>> Bill Burke
>>> JBoss, a division of Red Hat
>>> http://bill.burkecentral.com
>>
>
> --
> Bill Burke
> JBoss, a division of Red Hat
> http://bill.burkecentral.com