[noPoll] Stress Test -- Final Flag after calling nopoll_conn_get_msg
Kale, Rahul
Rahul.Kale en barco.com
Jue Jul 14 00:14:08 CEST 2016
Hello,
Were you able to reproduce any of the issues I had raised recently?
I would like to know if I am using the noPoll library in the wrong fashion.
Regards,
Rahul
-----Original Message-----
From: Kale, Rahul
Sent: Tuesday, June 14, 2016 9:51 AM
To: nopoll en lists.aspl.es
Cc: francis.brosnan en aspl.es
Subject: RE: Stress Test -- Final Flag after calling nopoll_conn_get_msg
Hello,
Any update on this issue yet?
On second thoughts, I believe that noPoll library should also attempt to consolidate multiple fragments into a complete message when
nopoll_conn_get_msg() is used.
A typical use case for websockets is to send JSON or XML text messages back and forth. Forcing the user to worry about coding up stitching together potentially fragmented messages is an unnecessary burden. Websockets specifically provides a way to have a message based API and indicate end of message using final bit.
The noPoll library already provides a stream based API. The user should be able to pick the appropriate API (message based vs. stream based) depending on the use case. Presumably the user of the library is fully aware about the nature of messages transferred and the expected size of the messages. To prevent blow up of the memory, the library should expose configurable parameters for maximum size of each message and total size of fragmented messages. Once these are exceeded, it should be OK for the noPoll library to abort the connection. The user can configure these parameters based on domain knowledge and tolerance for memory consumption.
I noticed that other websocket libraries already follow this approach.
For example:
https://github.com/theturtle32/WebSocket-Node/blob/master/docs/WebSocketClient.md
Best Regards,
Rahul
> -----Original Message-----
> From: Kale, Rahul
> Sent: Friday, June 03, 2016 1:22 PM
> To: nopoll en lists.aspl.es
> Cc: francis.brosnan en aspl.es
> Subject: RE: Stress Test -- Final Flag after calling
> nopoll_conn_get_msg
>
>
>
>
> Hello,
>
> I believe I have a resolution for the incorrect final flag reported on
> the message returned by nopoll_conn_get_msg();
>
> First off, I determined that this is a receiver side issue since the
> problem was reproducible even with the Node.js version of the server side.
>
> From the documentation for noPoll:
>
> "In general noPoll tries to consolidate internal frame fragments but
> only for some particular functions like (nopoll_conn_get_msg). For example, ..."
>
> Looking at the code for nopoll_conn_get_msg() I believe that this is
> not what is being done. When a partial frame (header and part of the
> body) is received, it is handed back directly to the caller. It is
> made to look like a fragmented message with adjustments made for
> is_fragment() and is_final().
>
> Indeed your approach is reasonable. We should NOT attempt to
> automatically integrate fragments into a single message. Otherwise
> there is a danger of running out of memory. On the wire, a sender is
> free to send an arbitrarily long message. Potentially, an infinitely
> long stream of fragmented messages can be send with a 'final' flag set
> after even gigabytes. I believe currently there is no upper limit set
> on the size of a complete message in the Websocket specification.
>
> The adjustments made for is_final() flag for partial frame received
> did not seem to be correct. I have attempted to fix this by doing the
> following changes. First, I introduced a new 'is_fin' flag in the noPollMsg structure.
> I ensure that the original 'has_fin' flag is only initialized from the
> first header received. That flag is transfered to the next message
> structure created in case the previous one was partial frame. The new
> 'is_fin' flag is set to false when delivering a partial frame. It is
> set to the original 'has_fin' value when the final portion of the
> frame is delivered. Finally, the nopoll_msg_is_final() method now returns the new 'is_fin' flag instead of the 'has_fin' flag.
>
> The reasoning is that the original incoming message frame might also
> be itself fragmented and the value for the final flag in the last
> segment delivered needs to reflect that.
>
> After the above changes, the reported is_final() flag is always
> correct for all messages received even under stress.
>
> I do not have a full understanding of the code so you may have a
> better approach to fix this. Anyway, below is a unified diff of the changes I made:
>
>
> diff -u -r -w ../tmp/nopoll-0.4.1.b265/src/nopoll_conn.c ./src/nopoll_conn.c
> --- ../tmp/nopoll-0.4.1.b265/src/nopoll_conn.c 2016-05-23
> 12:01:54.000000000 -0700
> +++ ./src/nopoll_conn.c 2016-06-03 12:06:28.502322481 -0700
> @@ -2752,7 +2752,8 @@
> msg->is_fragment = nopoll_true;
>
> /* get fin bytes */
> - msg->has_fin = 1; /* for now final message */
> + msg->has_fin = conn->previous_msg->has_fin;
> + msg->is_fin = nopoll_false;
> msg->op_code = 0; /* continuation frame */
>
> /* copy initial mask indication */ @@ -3069,11 +3070,16 @@
>
> /* flag this message as a fragment */
> msg->is_fragment = nopoll_true;
> + msg->is_fin = nopoll_false;
>
> /* flag that this message doesn't have FIN = 0 because
> * we wasn't able to read it entirely */
> /* msg->has_fin = 0; */
> +
> } /* end if */
> + else {
> + msg->is_fin = msg->has_fin;
> + }
>
> /* flag the message was being a fragment according to previous flag
> */
> msg->is_fragment = msg->is_fragment || conn-
> >previous_was_fragment || msg->has_fin == 0;
>
> diff -u -r -w ../tmp/nopoll-0.4.1.b265/src/nopoll_msg.c ./src/nopoll_msg.c
> --- ../tmp/nopoll-0.4.1.b265/src/nopoll_msg.c 2015-11-15
> 01:57:52.000000000 -0800
> +++ ./src/nopoll_msg.c 2016-06-03 11:52:28.102344864 -0700
> @@ -180,7 +180,7 @@
> if (msg == NULL)
> return nopoll_false;
>
> - return msg->has_fin;
> + return msg->is_fin;
> }
>
> /**
>
> diff -u -r -w ../tmp/nopoll-0.4.1.b265/src/nopoll_private.h
> ./src/nopoll_private.h
> --- ../tmp/nopoll-0.4.1.b265/src/nopoll_private.h 2016-04-21
> 01:18:52.000000000 -0700
> +++ ./src/nopoll_private.h 2016-06-03 11:25:52.000716385 -0700
> @@ -358,6 +358,7 @@
> int remain_bytes;
>
> nopoll_bool is_fragment;
> + nopoll_bool is_fin;
> int unmask_desp;
> };
>
>
>
> Regards,
>
> Rahul
>
>
>
> Rahul Kale
>
> IP Video Systems
> Barco, Inc
> 1287 Anvilwood Ave
> Sunnyvale, CA 94089
>
> Tel +1 408 400 4238
This message is subject to the following terms and conditions: MAIL DISCLAIMER<http://www.barco.com/en/maildisclaimer>
Más información sobre la lista de distribución noPoll