[RESTEASY-2196] CompletionStageRxInvokerImpl should not block on a th…

[anilgursel]

…read.

More details can be found at https://issues.jboss.org/projects/RESTEASY/issues/RESTEASY-2196

[asoldano]

@anilgursel thanks for your contribution. I haven't looked at the details yet, however I see that all test runs have been aborted after having reached the maximum allowed time. Is the build, including full testsuite passing there? Do you have any clue? Thanks

[anilgursel]

I am actually not happy with adding response.bufferEntity() here. This will copy the entity to memory. It may be unnecessary for the scenarios where entity is discarded. Also, it would prevent entity streaming as well.

The Javadoc of completed method in InvocationCallback reads:

* Once this invocation callback method returns, the underlying {@link javax.ws.rs.core.Response}
* instance will be automatically closed by the runtime.

Without buffering, doTestRxClientGet fails at

CompletableFuture<Response> cs = (CompletableFuture<Response>) invoker.get();
String response = cs.get().readEntity(String.class);

Because, by the time readEntity is called the Response is already closed. This can be addressed by changing the code as

 String response = cs.thenApply(r -> r.readEntity(String.class)).get();

This would allow the readEntity to be called before InvocationCallback completes. This is quite limiting because it requires all chaining to be done on the CompletableFuture and without any *Async methods. This, for instance, causes problem for SingleRxInvoker which is built on top of CompletionStageRxInvoker. In RxTest.java:

@Test
public void testSingle() throws Exception {
   Single<Response> single = client.target(generateURL("/single")).request().rx(SingleRxInvoker.class).get();
   single.subscribe((Response r) -> {value.set(r.readEntity(String.class)); latch.countDown();});
   latch.await();
   assertEquals("got it", value.get());
}

By the time r.readEntity(String.class) is called the Response is already closed (per InvocationCallback javadoc). It throws an exception and gets stuck (this was the problem that caused timeouts for the build earlier.).

Calling response.bufferEntity() in completed of InvocationCallback lets this code run fine. But, I am actually not sure how or is it even reliable. The Javadoc of bufferEntity reads:

* {@code readEntity(...)} methods on the response instance. Note however, that
* once the response instance itself is {@link #close() closed}, the implementations
* are expected to release the buffered message entity data too. Therefore any subsequent
* attempts to read a message entity stream on such closed response will result in an
* {@link IllegalStateException} being thrown.

Any suggestion to make this reliable? Or we cannot use InvocationCallback at all?

[anilgursel]

@asoldano Do you have any suggestions to get around this problem?

[anilgursel]

@asoldano I see your comment here: jakartaee/rest#626 (comment). And that is actually what I am relying on. However, with that, I cannot find a reliable way of using .async in CompletionStageRxInvokerImpl as you can see in this PR.

Please share your feedback and suggestions, and I am willing to contribute further. But, at this point, I hit a roadblock with this PR.

[anilgursel]

I see all three implementations are blocking on a thread for CompletionStageRxInvoker:

• RestEasy - CompletionStageRxInvokerImpl.java#L43
• Jersey - JerseyCompletionStageRxInvoker.java#L45
• CXF - CompletionStageRxInvokerImpl.java#L149

Is there any way to use Reactive Clients without actually blocking on a thread?  The discussion at jakartaee/rest#626 does not seem to provide a concrete solution. One of the last comments reads:

The spec does not impose any NIO requirements. If an implementation can do things non-blockingly, more power to it.

If we cannot use a JAX-RS impl with NIO requirements, than the large portion of the rx invoker value is lost.

Can you please suggest how one can use JAX-RS RxInvoker APIs and not block in RestEasy? 

[anilgursel]

I can work on a PR to add an API to AsyncClientHttpEngine to return CompletableFuture. The method could have a default implementation to call the submit method that returns a Future and block on it.

Would that be something you would be open to consider?

[anilgursel]

Created a separate PR for this issue. Please see #1988.