[jira] [Comment Edited] (CXF-7575) @Suspended race condition

["John Bellassai (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CXF-7575?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16344444#comment-16344444 ] 

John Bellassai edited comment on CXF-7575 at 1/30/18 3:25 AM:
--------------------------------------------------------------

Sergey,

Yes, your breakdown is exactly the situation I was trying to describe.  

I followed the trail of CXF-7037 and found [this|https://www.mail-archive.com/dev@tomcat.apache.org/msg110682.html] thread in the Tomcat mailing list in which the bug mentioned in CXF-7037 is discussed.

From what I can tell, the issue in Tomcat was resolved at least in version 8.5.6 (2016-10-10). Here is the relevant paragraph from the [changelog|https://tomcat.apache.org/tomcat-8.5-doc/changelog.html]: 
{quote}Refactor the code that implements the requirement that a call to complete() or dispatch() made from a non-container thread before the container initiated thread that called startAsync() completes must be delayed until the container initiated thread has completed. Rather than implementing this by blocking the non-container thread, extend the internal state machine to track this. This removes the possibility that blocking the non-container thread could trigger a deadlock. (markt)
{quote}
So, this would lead me to believe that it is safe to synchronize the _doResumeFinal_ method now, at least as far as Tomcat is concerned.


was (Author: jbellassai):
Sergey,

Yes, your breakdown is exactly the situation I was trying to describe.  

I followed the trail of CXF-7037 and found [this|https://www.mail-archive.com/dev@tomcat.apache.org/msg110682.html] thread in the Tomcat mailing list in which the bug mentioned in CXF-7037 is discussed.

From what I can tell, the issue in Tomcat was resolved at least in version 8.5.6 (2016-10-10). Here is the relevant paragraph from the [changelog|[https://tomcat.apache.org/tomcat-8.5-doc/changelog.html]:] 
{quote}Refactor the code that implements the requirement that a call to complete() or dispatch() made from a non-container thread before the container initiated thread that called startAsync() completes must be delayed until the container initiated thread has completed. Rather than implementing this by blocking the non-container thread, extend the internal state machine to track this. This removes the possibility that blocking the non-container thread could trigger a deadlock. (markt)
{quote}
So, this would lead me to believe that it is safe to synchronize the _doResumeFinal_ method now, at least as far as Tomcat is concerned.

> @Suspended race condition
> -------------------------
>
>                 Key: CXF-7575
>                 URL: https://issues.apache.org/jira/browse/CXF-7575
>             Project: CXF
>          Issue Type: Bug
>          Components: JAX-RS
>    Affects Versions: 3.1.14
>            Reporter: John Bellassai
>            Priority: Major
>         Attachments: CXF-7575.patch
>
>
> There appears to be a race condition with the use of AsyncResponseImpl where my user thread can invoke resume() before initialSuspend is set to false by suspendContinuationIfNeeded() and therefore the resume() call does not actually resume the Continuation _and returns true_, indicating that the resume was successful even though it wasn't.
> I've spent all day trying to make sense of this problem and my understanding of how all of this works together is still a bit spotty, but it seems to me that AsyncResponseImpl.suspendContinuationIfNeeded() (or something similar) should be called _before_ invoking the JAXRS method. Right now, that method is only called after the JAXRS method is invoked by JAXRSInvoker so the instance of AsyncResponse passed into the JAXRS method appears to not actually get suspended (or perhaps _marked_ internally as suspended) until after the JAXRS method returns.  If my async task happens to get finished very quickly and calls resume() before that happens, it fails silently.
> I seem to be able to circumvent this problem by running the following at the start of my JAXRS method (pseudo code):
> {code}
> @POST
> @Path(....)
> void myJaxrsMethod(@Suspended AsyncResponse asyncResponse, ...) {
>     if(asyncResponse instanceof AsyncResponseImpl) {
>         ((AsyncResponseImpl)asyncResponse).suspendContinuationIfNeeded()
>     }
>     Runnable asyncTask = createAsyncTask(asyncResponse)
>     submitAsyncTask(asyncTask)
> }
> {code}
> which is why I suspect suspendContinuationIfNeeded() should be called before JAXRSInvoker invokes the JAXRS method.
> One of the things that made this really difficult to track down was that AsyncResponseImpl.resume() returns true even if the Continuation was not resumed! If you make it into doResumeFinal(), like was happening in my case, the return is always true even if cont.resume() is not called. So from user code, it looks like everything is ok, but the response never gets sent to the client.
> This seems somewhat related to the problems reported in CXF-7037



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)