[
https://issues.apache.org/jira/browse/TUSCANY-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Greg Dritschler updated TUSCANY-3940:
-------------------------------------
Description:
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.
was:
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 from
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.
> 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
