[noPoll] Stress Test -- Final Flag after calling nopoll_conn_get_msg

[Kale, Rahul]

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>