[vlc-commits] https: drain HTTP/2 output queue if possible

[Rémi Denis-Courmont]

vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Sun Dec 20 23:29:51 2015 +0200| [8bff13adf437e716d72952d5a7a3e7f07f8cb357] | committer: Rémi Denis-Courmont

https: drain HTTP/2 output queue if possible

The underlying TCP socket is mostly used for receiving. So in most
cases, there is enough space in send buffers for the final packets
(such as RST_STREAM and GOAWAY frames).

This fixes a race condition where the output thread got cancelled
before draining the queue. Now, it only gets cancelled if the send
buffer is congested (i.e. it calls poll(POLLOUT)). In normal cases,
the thread will exit explicitly after draining.

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=8bff13adf437e716d72952d5a7a3e7f07f8cb357
---

 modules/access/http/h2conn.c   |    1 -
 modules/access/http/h2output.c |   62 +++++++++++++++++++++++++---------------
 2 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/modules/access/http/h2conn.c b/modules/access/http/h2conn.c
index 535ecbe..c982a53 100644
--- a/modules/access/http/h2conn.c
+++ b/modules/access/http/h2conn.c
@@ -650,7 +650,6 @@ static void vlc_h2_conn_destroy(struct vlc_h2_conn *conn)
 {
     assert(conn->streams == NULL);
 
-    /* TODO: properly try to drain pending data in output */
     vlc_h2_error(conn, VLC_H2_NO_ERROR);
 
     vlc_cancel(conn->thread);
diff --git a/modules/access/http/h2output.c b/modules/access/http/h2output.c
index a041daf..6cd7085 100644
--- a/modules/access/http/h2output.c
+++ b/modules/access/http/h2output.c
@@ -45,10 +45,11 @@ struct vlc_h2_output
 {
     struct vlc_tls *tls;
 
-    struct vlc_h2_queue prio;
-    struct vlc_h2_queue queue;
-    size_t size;
-    bool failed;
+    struct vlc_h2_queue prio; /*< Priority send queue */
+    struct vlc_h2_queue queue; /*< Normal send queue */
+    size_t size; /*< Total queues depth (bytes) */
+    bool failed; /*< Connection failure flag */
+    bool closing; /*< Connection shutdown pending flag */
 
     vlc_mutex_t lock;
     vlc_cond_t wait;
@@ -126,7 +127,6 @@ static struct vlc_h2_frame *vlc_h2_output_dequeue(struct vlc_h2_output *out)
     size_t len;
 
     vlc_mutex_lock(&out->lock);
-    mutex_cleanup_push(&out->lock);
 
     for (;;)
     {
@@ -138,7 +138,15 @@ static struct vlc_h2_frame *vlc_h2_output_dequeue(struct vlc_h2_output *out)
         if (q->first != NULL)
             break;
 
+        if (unlikely(out->closing))
+        {
+            vlc_mutex_unlock(&out->lock);
+            return NULL;
+        }
+
+        int canc = vlc_savecancel();
         vlc_cond_wait(&out->wait, &out->lock);
+        vlc_restorecancel(canc);
     }
 
     frame = q->first;
@@ -154,7 +162,6 @@ static struct vlc_h2_frame *vlc_h2_output_dequeue(struct vlc_h2_output *out)
     assert(out->size >= len);
     out->size -= len;
 
-    vlc_cleanup_pop();
     vlc_mutex_unlock(&out->lock);
 
     frame->next = NULL;
@@ -244,26 +251,27 @@ static void *vlc_h2_output_thread(void *data)
     struct vlc_h2_output *out = data;
     struct vlc_h2_frame *frame;
 
-    do
+    while ((frame = vlc_h2_output_dequeue(out)) != NULL)
     {
-        frame = vlc_h2_output_dequeue(out);
         vlc_h2_frame_dump(out->tls->obj, frame, "out");
-    }
-    while (vlc_h2_frame_send(out->tls, frame) == 0);
 
-    vlc_mutex_lock(&out->lock);
-    /* The connection can fail asynchronously. For the sake of simplicity, the
-     * caller will be notified only on the next attempt to send something. */
-    out->failed = true;
-    /* At this point, the caller will not touch the queue at all - until this
-     * thread terminates. So the lock is no longer needed here. */
-    vlc_mutex_unlock(&out->lock);
-    /* Lets not retain frames in memory useless in the mean time. */
-    vlc_h2_output_flush_unlocked(out);
-    out->prio.first = NULL;
-    out->prio.last = &out->prio.first;
-    out->queue.first = NULL;
-    out->queue.last = &out->queue.first;
+        if (vlc_h2_frame_send(out->tls, frame))
+        {   /* The connection failed asynchronously. The caller will be
+             * notified at the next attempt to queue (as with TCP sockets). */
+            vlc_mutex_lock(&out->lock);
+            out->failed = true;
+            vlc_mutex_unlock(&out->lock);
+
+            /* The caller will leave the queues alone from now on until this
+             * thread ends. The queues are flushed to free memory. */
+            vlc_h2_output_flush_unlocked(out);
+            out->prio.first = NULL;
+            out->prio.last = &out->prio.first;
+            out->queue.first = NULL;
+            out->queue.last = &out->queue.first;
+            break;
+        }
+    }
 
     return NULL;
 }
@@ -298,6 +306,7 @@ struct vlc_h2_output *vlc_h2_output_create(struct vlc_tls *tls, bool client)
     out->queue.last = &out->queue.first;
     out->size = 0;
     out->failed = false;
+    out->closing = false;
 
     vlc_mutex_init(&out->lock);
     vlc_cond_init(&out->wait);
@@ -316,11 +325,18 @@ struct vlc_h2_output *vlc_h2_output_create(struct vlc_tls *tls, bool client)
 
 void vlc_h2_output_destroy(struct vlc_h2_output *out)
 {
+    vlc_mutex_lock(&out->lock);
+    out->closing = true;
+    vlc_cond_signal(&out->wait);
+    vlc_mutex_unlock(&out->lock);
+
     vlc_cancel(out->thread);
     vlc_join(out->thread, NULL);
 
     vlc_cond_destroy(&out->wait);
     vlc_mutex_destroy(&out->lock);
+    /* Flush queues in case the thread was terminated within poll() and some
+     * packets were still queued. */
     vlc_h2_output_flush_unlocked(out);
     free(out);
 }