On Oct 2, 2012, at 2:04 AM, Sergey Beryozkin <sberyozkin_at_talend.com> wrote:
> On 01/10/12 21:40, Marek Potociar wrote:
>>
>> On Oct 1, 2012, at 3:59 AM, Sergey Beryozkin<sberyozkin_at_talend.com> wrote:
>>
>>> On 28/09/12 19:15, Marek Potociar wrote:
>>>>
>>>> On Sep 27, 2012, at 5:56 AM, Sergey Beryozkin<sberyozkin_at_talend.com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> I can see AsyncResponse.resume throws IllegalStateException if the invocation has not been suspended.
>>>>
>>>> I was thinking of updating the API earlier to return boolean success value instead of throwing exceptions from resume/cancel methods and forgot to make the change. That's more user-friendly than throwing an exception.
>>>>>
>>>>> IMHO it is too much of a restriction, possibly an implementation specific one.
>>>>
>>>> When you inject the AsyncResponse it is formally suspended. So the first invocation of resume() is always ok (unless the async response has been cancelled already).
>>>> The exception thus may only be expected in cases when someone else already resumed or cancelled the response earlier. I don't see anything implementation specific about that. I could however agree that returning boolean instead of throwing an exception may be more user-friendly.
>>>
>>> Regarding returning boolean vs throwing an exception.
>>> I think it does not make a difference.
>>>
>>> This is because the runtime effectively guarantees that the application level code (original resource method accepting AsyncResponse or TimeoutHandler) does always see a 'suspended' AsyncResponse.
>>
>> The above would require an extra synchronization in the framework, which is not what we want IMO.
>
> What does that mean ? Are you saying the application code calling either of AsyncResponse methods affecting the execution and which can only work if AsyncResponse in a suspended mode, can face a case where resume/cancel/timeout does not work ?
I am saying that the code like the one bellow is not thread-safe without extra synchronization. Which is why I believe, that returning a boolean value indicating a success is perhaps better than throwing a runtime exception.
if (asyncResponse.isSuspended()) {
// race-condition hazard
// the connection may get resumed by another thread
// between the two calls.
asyncResponse.resume(timoutResponse);
}
Cheers,
Marek
>
> Cheers, Sergey
>
>>
>>>
>>> However, after saying the above, I guess returning 'boolean' and updating the docs to simply say that say "resume(...) -> Resumes the suspended invocation, timeout(...) -> extends the suspended invocation, cancel(...) - cancels the suspended invocation" would probably be the right thing to do
>>
>> Ok, so I guess even if we're coming from different directions, we're arriving to the same conclusion.
>>
>> Marek
>>>
>>> Sergey
>>>
>>>
>>>>>
>>>>> IMHO, the following should work
>>>>>
>>>>> @GET
>>>>> void getIt(AsyncResponse response) {
>>>>> response.resume(myObject);
>>>>> }
>>>>
>>>> IMHO, the above should not work, unless you use @Suspended annotation. Because since JAX-RS 1.0, the first non-anotated parameter in a resource method is a request entity. AsyncResponse is not a request entity.
>>>>
>>>> If one uses the @Suspended annotation, then the example should work. Because, at least formally, the AsyncResponse has been suspended. What optimizations you do in your implementation is, of course up, to you.
>>>>
>>>>>
>>>>> if 'myObject' happens to be available immediately.
>>>>> The runtime has to suspend only after "getIt" returns, but only if no response object is already available.
>>>>
>>>> Now this is an implementation detail IMO. :)
>>>>
>>>> Marek
>>>>
>>>>
>>>>>
>>>>> Thoughts ?
>>>>>
>>>>> Sergey
>>>>
>>>
>>>
>>
>
>