[ https://issues.apache.org/jira/browse/TUSCANY-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13100220#comment-13100220 ]
ant elder commented on TUSCANY-3940: ------------------------------------ Patch applied, thanks for the fix Greg. I've added an exclude to the JCA compliance test runner for JCA_11017 and will follow up with OASIS to get the test fixed > 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 > 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