[jira] [Closed] (TUSCANY-3940) Change AsyncJDKInvocationHandler to use WorkScheduler directly

["Simon Laws (JIRA)" <de...@tuscany.apache.org>]

     [ https://issues.apache.org/jira/browse/TUSCANY-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Simon Laws closed TUSCANY-3940.
-------------------------------

    Resolution: Fixed

exclude for 11017 now removed

> Change AsyncJDKInvocationHandler to use WorkScheduler directly
> --------------------------------------------------------------
>
>                 Key: TUSCANY-3940
>                 URL: https://issues.apache.org/jira/browse/TUSCANY-3940
>             Project: Tuscany
>          Issue Type: Improvement
>    Affects Versions: Java-SCA-2.0
>            Reporter: Greg Dritschler
>            Priority: Minor
>             Fix For: Java-SCA-2.0
>
>         Attachments: TUSCANY-3940.patch
>
>
> AsyncJDKInvocationHandler obtains an ExecutorService from the WorkScheduler utility and uses it to schedule asynchronous requests.  AsyncJDKInvocationHandler could just as well use the WorkScheduler utility directly.  This might simplify things for some WorkScheduler providers since they wouldn't have to support an ExecutorService interface.  I am attaching a patch to make this change.
> While working on this, I discovered some other problems which I'm also fixing in this patch.
> 1.  When the client passes a callback object, AsyncJDKInvocationHandler.doInvokeAsyncCallback() invokes the callback immediately after making the asynchronous request.  The callback object should be invoked when the response is actually available.  I am fixing this by saving the callback object in the future object, AsyncInvocationFutureImpl.  When the response or fault is delivered to the future (via setResponse() or setFault() or setWrappedFault()), the future invokes the callback object.  
> 2.  AsyncJDKInvocationHandler.separateThreadInvoker.run() contains a catch block for a ServiceRuntimeException.  If the cause of the ServiceRuntimeException is NOT a FaultException, the exception is swallowed for no reason.  I changed the code to deliver the exception to the future.
> 3.  AsyncJDKInvocationHandler.doInvokeSync() contains a wait of 1000 seconds when a client does a sync invocation of an async service.  This is way too long for a synchronous timeout.  Ideally this should be configurable in some manner, although I don't know how much effort we want to spend on it given that this is not a desirable invocation pattern.  For now I am changing it to wait 120 seconds.
> 4. AsyncFaultWrapper tries to reconstruct a fault using a constructor with a String argument.  If the class does not have such a constructor, this fails.  I have added code to try with the default constructor.
> Change #1 exposed an issue in the async-services itest.  Service1ClientImpl uses static variables for the async response and async response fault.  After the first time through, these values may have residual values from the prior execution and the test will skip waiting on the mutex.  The test got away with this before because of the bad logic in AsyncJDKInvocationHandler:
>   -- AsyncJDKInvocationHandler.doInvokeAsyncCallback() scheduled a runnable to invoke the request.  It waited for this runnable to complete!
>   -- The runnable invoked the callback handler after firing the request.  The itest callback handler does a get() on the provided future with an indefinite timeout. 
> So the itest was guaranteed that its static variables would be updated.  The test was really running pseudo-synchronously.  I changed the itest to use non-static variables.  Now it is falling into the mutex wait for a new response.  I increased the wait time a bit just to be safe.
> *** The JCA_11017 compliance test has a very similar problem.  It is not using statics, but it is making multiple requests inside a loop using the same variables.  It uses a CountdownLatch with a value of 1, so after the first request the latch won't wait anymore.  This causes it to not wait for the exception response for the second request.  It got away with this before for the same reason outlined above;  the requests were actually pseudo-synchronous.  The patch does not have a fix for this, so JCA_11017 will fail until it is somehow corrected.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira