WebSocket client API

[Simone Bordet]

Hi,

On Tue, Oct 20, 2015 at 11:07 AM, Peter Levart <peter.levart at gmail.com> wrote:
> Hi,
>
> Another thought about [Char|Byte]Buffer recycling. If the onXXX call-backs
> returned a CF so that they could asynchronously signal when they are done
> with consumption, the signature of the method could be:
>
> CompletionStage<CharBuffer> onText(..., CharBuffer cb, ...)
>
> Implementor of the method (the user) could do the following things:
>
> - return null or CompletableFuture.completedStage(null) to signal that the
> method already performed the consumption synchronously, but that it retained
> the CharBuffer, so WebSocket should not recycle it, but must consider it
> lost.
> - return CompletableFuture.completedStage(cb) to signal that the method
> already performed the consumption synchronously and the result of completed
> CompletionStage is the buffer that can be returned into the internal pool of
> WebSocket buffers.
> - return a CompletionStage that is yet to be completed asynchronously with
> the 'cb' or null as the result. The first case returns the buffer to the
> pool, the 2nd signals to WebSocket that the buffer is lost.
>
> This way, buffer recycling is in the domain of WebSocket implementation -
> not the user of WebSocket.
>
> The return type of onXXX methods should be CompletionStage not
> CompletableFuture. CompletionStage is not a Future and does not have methods
> that allow canceling of the underlying computation. All that WebSocket needs
> is attaching a completion that recycles the buffer, like:
>
> CharBuffer message = ... get buffer from pool or create new one ...
>
> ... fill message with data ...
>
> CompletionStage<CharBuffer> cs = listener.onText(..., message, ...);
>
> if (cs != null) {
>     cs.thenAccept(cb -> {
>         if (cb != null) {
>             .... return cb to buffer pool ...
>         }
>     });
> }
>
> What do you think?

The ability to return null to be completely equivalent to returning
CompletableFuture.completedFuture(cb) would cover a common case
(synchronous consumption) without incurring in forced allocation.
I am not sure how common is the case to synchronously consume the
buffer, but then not returning it.
Perhaps your cases could be reduced to:

* return null or CompletableFuture.completedFuture(cb) to indicate
full recycle of the buffer.
* return CompletableFuture.completedFuture(null) to indicate that the
buffer is lost.

This would reduce the allocation cost to just the fully async consumption case.

With your proposal the typical async proxy case would then write from:

onText(WebSocket ws, CharBuffer cb, boolean isLast)
{
    return ws.sendText(cb, isLast).thenAccept(_ -> ws.request(1));
}

to:

onText(WebSocket ws, CharBuffer cb, boolean isLast)
{
    return ws.sendText(cb, isLast).thenApply(_ -> { ws.request(1); return cb });
}

Just slightly more complex.

However, would it not be a potential attack vector ?
Applications would be able to "inject" a buffer into the
implementation buffer pool, and be able to peek/modify bytes from
other connections.
It should be:

if (cs != null) {
    cs.thenAccept(cb -> {
        if (cb != null) {
            .... return *message* to buffer pool ...
        }
    });
}

At that point, the return value of onText() could well be a CF<?> like
it already is, no ?
The implementation would just need to test for null, but then never
actually use the returned object wrapped by the CF.

What do you think ?

-- 
Simone Bordet
http://bordet.blogspot.com
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz