users@websocket-spec.java.net

[jsr356-users] [jsr356-experts] Re: Clarification required on sending messages concurrently

From: Stuart Douglas <stuart.w.douglas_at_gmail.com>
Date: Mon, 12 Jan 2015 20:37:02 +0000

---------- Forwarded message ---------
From: Stuart Douglas <stuart.w.douglas_at_gmail.com>
Date: Tue Jan 13 2015 at 7:34:53 AM
Subject: Re: [jsr356-users] [jsr356-experts] Re: Clarification required on
sending messages concurrently
To: <jsr356-experts_at_websocket-spec.java.net>


On Tue Jan 13 2015 at 5:53:30 AM Pavel Bucek <pavel.bucek_at_oracle.com> wrote:

> Hi Mark,
>
> seems like there might be some misinterpretation in the original thread
> - I read the reply from Danny as "container has to send the whole
> message before sending another one"; it does not seem to concern async
> messages, because container already has the message and it will be sent
> (or @OnError invoked).
>
> Your sample can be reduced to:
>
> session.getAsyncRemote().sendText(message);
> session.getBasicRemote().sendText(message);
>
>
> and I don't think this should end up in an exception. There must be some
> kind of queue implemented in the container and second call will wait for
> first message to be sent, but that's all. Consider Session class javadoc:
>
> * Session objects may be called by multiple threads. Implementations must
> * ensure the integrity of the mutable properties of the session under
> such circumstances.
>
>
> There is no API for users to get the state of current Session instance
> like "SENDING" or "READY_TO_SEND". When shared and used from multiple
> threads, it would not be easily possible to reliably send a message, if
> the sending state of individual messages could cause an exception.
>

I think the problematic case here is code something like the following
contrived example:

for(int i = 0; i < 10000; ++i) {
  session.getAsyncRemote().sendText("Hello " + i);
}

If the connection is not fast enough to send all these the container will
have to buffer them, and in some situations the container may end up having
to buffer a lot of data (a more realistic case would simply be a slow
connection, and the server starts generating messages faster than they can
be sent to the client).


> The other thing is the original usecase. I believe we should signal
> something when user tries to send a message in the middle of sending
> another partial message. I will have to investigate little bit more
> about this, but I can say that the RI does not do this at the moment.
>
> Looking at this issue from broader perspective, it could be nice to have
> the alternative to this, something as "force send", since we might get
> to the state when we want to send some message, no matter what is
> happening in other threads. Just for the sake of completeness, here is
> the usecase:
>
> session.getBasicRemote().sendText(message, false); // partial
> session.getBasicRemote().sendText(message);


> (Currently, RI sends the second (whole) message as part of the previous
> one; or not really, it is just interpreted in that way..).
>

Looking at our (Undertow's) implementation of this it looks like this
example would block indefinitely, or at least until another thread calls

session.getBasicRemote().sendText(message, true);

In this case our underlying websocket implementation will delay sending the
second message until the first is done, so as blocking IO is in use it
would require another thread to finish sending the first message.

Regarding your 'Force Send' idea I am not sure how workable it is, as if
any part of the first message has already hit the wire you have to wait for
it to be completed. I guess if you have a few messages queued it could move
the message to the front of the queue.


> Following is even more evil:
>
> session.getBasicRemote().sendText(m, false); // partial text
> session.getBasicRemote().sendBinary(ByteBuffer.wrap(m.getBytes()));
>
> (this is interpreted in the same way as previous; if you think of the
> frame content and order, it makes sense that the other side interpreted
> it as one text message. Problem is that we might need to detect this on
> the sender side and throw an exception probably..)
>

We currently throw an exception in this case, although there is no real
reason why we could not just hold off sending the binary message till the
first one is complete.

Stuart


>
>
> Thanks and regards,
> Pavel
>
>
> On 12/01/15 11:21, Mark Thomas wrote:
> > Looking back though the EG discussions, we discussed concurrency and
> > agreed that one message had to complete before the next was sent [1]. No
> > distinction was made between synchronous and asynchronous messages.
> >
> > The discussion in [1] happened before the split between
> > RemoteEndpoint.[Basic|Async] and after the split restriction described
> > in [1] only appears in the Javadoc for RemoteEndpoint.Basic.
> >
> > My question is this:
> >
> > Does the restriction agreed in [1] apply to async messages or not?
> >
> > I believe that the restriction does apply. Without it, there is no
> > mechanism for the container to signal to the application to stop sending
> > messages if the client isn't reading them fast enough.
> >
> > By way of example, consider the following:
> > @OnMessage
> > public String onMessage(String message, Session session) {
> > received.add(message);
> > session.getAsyncRemote().sendText(MESSAGE_ONE);
> > return message;
> > }
> >
> >
> > My expectation is that, unless the async message completes very quickly
> > (unlikely) then an ISE will be thrown because the async message is still
> > in progress when the sync message send is triggered by the return value.
> >
> > That said, if batching is enabled then it is much more likely that the
> > async message would complete instantly and that the sync message would
> > then be sent.
> >
> > For bonus points, assuming that the batch buffer is big enough for both
> > the async and sync message, is the use of the return value to send the
> > message a trigger to flush the batch? I can seen no reason why this
> > should be the case but the question popped up on a Tomcat list so I
> > wanted to check here.
> >
> > Mark
> >
> >
> > [1]
> > https://java.net/projects/websocket-spec/lists/jsr356-expert
> s/archive/2013-02/message/48
> >
>