Commit 8bff13ad authored by Rémi Denis-Courmont's avatar 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.
parent 0ac52736
...@@ -650,7 +650,6 @@ static void vlc_h2_conn_destroy(struct vlc_h2_conn *conn) ...@@ -650,7 +650,6 @@ static void vlc_h2_conn_destroy(struct vlc_h2_conn *conn)
{ {
assert(conn->streams == NULL); assert(conn->streams == NULL);
/* TODO: properly try to drain pending data in output */
vlc_h2_error(conn, VLC_H2_NO_ERROR); vlc_h2_error(conn, VLC_H2_NO_ERROR);
vlc_cancel(conn->thread); vlc_cancel(conn->thread);
......
...@@ -45,10 +45,11 @@ struct vlc_h2_output ...@@ -45,10 +45,11 @@ struct vlc_h2_output
{ {
struct vlc_tls *tls; struct vlc_tls *tls;
struct vlc_h2_queue prio; struct vlc_h2_queue prio; /*< Priority send queue */
struct vlc_h2_queue queue; struct vlc_h2_queue queue; /*< Normal send queue */
size_t size; size_t size; /*< Total queues depth (bytes) */
bool failed; bool failed; /*< Connection failure flag */
bool closing; /*< Connection shutdown pending flag */
vlc_mutex_t lock; vlc_mutex_t lock;
vlc_cond_t wait; vlc_cond_t wait;
...@@ -126,7 +127,6 @@ static struct vlc_h2_frame *vlc_h2_output_dequeue(struct vlc_h2_output *out) ...@@ -126,7 +127,6 @@ static struct vlc_h2_frame *vlc_h2_output_dequeue(struct vlc_h2_output *out)
size_t len; size_t len;
vlc_mutex_lock(&out->lock); vlc_mutex_lock(&out->lock);
mutex_cleanup_push(&out->lock);
for (;;) for (;;)
{ {
...@@ -138,7 +138,15 @@ static struct vlc_h2_frame *vlc_h2_output_dequeue(struct vlc_h2_output *out) ...@@ -138,7 +138,15 @@ static struct vlc_h2_frame *vlc_h2_output_dequeue(struct vlc_h2_output *out)
if (q->first != NULL) if (q->first != NULL)
break; break;
if (unlikely(out->closing))
{
vlc_mutex_unlock(&out->lock);
return NULL;
}
int canc = vlc_savecancel();
vlc_cond_wait(&out->wait, &out->lock); vlc_cond_wait(&out->wait, &out->lock);
vlc_restorecancel(canc);
} }
frame = q->first; frame = q->first;
...@@ -154,7 +162,6 @@ static struct vlc_h2_frame *vlc_h2_output_dequeue(struct vlc_h2_output *out) ...@@ -154,7 +162,6 @@ static struct vlc_h2_frame *vlc_h2_output_dequeue(struct vlc_h2_output *out)
assert(out->size >= len); assert(out->size >= len);
out->size -= len; out->size -= len;
vlc_cleanup_pop();
vlc_mutex_unlock(&out->lock); vlc_mutex_unlock(&out->lock);
frame->next = NULL; frame->next = NULL;
...@@ -244,26 +251,27 @@ static void *vlc_h2_output_thread(void *data) ...@@ -244,26 +251,27 @@ static void *vlc_h2_output_thread(void *data)
struct vlc_h2_output *out = data; struct vlc_h2_output *out = data;
struct vlc_h2_frame *frame; 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"); vlc_h2_frame_dump(out->tls->obj, frame, "out");
}
while (vlc_h2_frame_send(out->tls, frame) == 0);
vlc_mutex_lock(&out->lock); if (vlc_h2_frame_send(out->tls, frame))
/* The connection can fail asynchronously. For the sake of simplicity, the { /* The connection failed asynchronously. The caller will be
* caller will be notified only on the next attempt to send something. */ * notified at the next attempt to queue (as with TCP sockets). */
out->failed = true; vlc_mutex_lock(&out->lock);
/* At this point, the caller will not touch the queue at all - until this out->failed = true;
* thread terminates. So the lock is no longer needed here. */ vlc_mutex_unlock(&out->lock);
vlc_mutex_unlock(&out->lock);
/* Lets not retain frames in memory useless in the mean time. */ /* The caller will leave the queues alone from now on until this
vlc_h2_output_flush_unlocked(out); * thread ends. The queues are flushed to free memory. */
out->prio.first = NULL; vlc_h2_output_flush_unlocked(out);
out->prio.last = &out->prio.first; out->prio.first = NULL;
out->queue.first = NULL; out->prio.last = &out->prio.first;
out->queue.last = &out->queue.first; out->queue.first = NULL;
out->queue.last = &out->queue.first;
break;
}
}
return NULL; return NULL;
} }
...@@ -298,6 +306,7 @@ struct vlc_h2_output *vlc_h2_output_create(struct vlc_tls *tls, bool client) ...@@ -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->queue.last = &out->queue.first;
out->size = 0; out->size = 0;
out->failed = false; out->failed = false;
out->closing = false;
vlc_mutex_init(&out->lock); vlc_mutex_init(&out->lock);
vlc_cond_init(&out->wait); vlc_cond_init(&out->wait);
...@@ -316,11 +325,18 @@ struct vlc_h2_output *vlc_h2_output_create(struct vlc_tls *tls, bool client) ...@@ -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) 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_cancel(out->thread);
vlc_join(out->thread, NULL); vlc_join(out->thread, NULL);
vlc_cond_destroy(&out->wait); vlc_cond_destroy(&out->wait);
vlc_mutex_destroy(&out->lock); 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); vlc_h2_output_flush_unlocked(out);
free(out); free(out);
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment