mirrors/gstreamer
mirrors/gstreamer
1
1
Fork 0
mirror of https://gitlab.freedesktop.org/gstreamer/gstreamer.git synced 2024-11-29 05:01:23 +00:00
Code Issues 1 Projects Releases Wiki Activity

adaptivedemux2: Do not submit_transfer when cancelled

Browse source 
There is a race condition where transfer has not been submitted yet while the
request is cancelled which leads to the transfer state going back to
`DOWNLOAD_REQUEST_STATE_OPEN` and the user of the request to get signalled about
its completion (and the task actually happening after it was cancelled) leading
to assertions and misbehaviours.

To ensure that this race can't happen, we start differentiating between the
UNSENT and CANCELLED states as in the normal case, when entering `submit_request`
the state is UNSENT and at that point we need to know that it is not because
the request has been cancelled.

In practice this case lead to an assertion in
`gst_adaptive_demux2_stream_begin_download_uri` because in a previous call to
`gst_adaptive_demux2_stream_stop_default` we cancelled the previous request and
setup a new one while it had not been submitted yet and then got a `on_download_complete`
callback called from that previous cancelled request and then we tried to do
`download_request_set_uri` on a request that was still `in_use`, leading to
something like:

```
 #0: 0x0000000186655ec8 g_assert (request->in_use == FALSE)assert.c:0
 #1: 0x00000001127236b8 libgstadaptivedemux2.dylib`download_request_set_uri(request=0x000060000017cc00, uri="https://XXX/chunk-stream1-00002.webm", range_start=0, range_end=-1) at downloadrequest.c:361
 #2: 0x000000011271cee8 libgstadaptivedemux2.dylib`gst_adaptive_demux2_stream_begin_download_uri(stream=0x00000001330f1800, uri="https://XXX/chunk-stream1-00002.webm", start=0, end=-1) at gstadaptivedemux-stream.c:1447
 #3: 0x0000000112719898 libgstadaptivedemux2.dylib`gst_adaptive_demux2_stream_load_a_fragment [inlined] gst_adaptive_demux2_stream_download_fragment(stream=0x00000001330f1800) at gstadaptivedemux-stream.c:0
 #4: 0x00000001127197f8 libgstadaptivedemux2.dylib`gst_adaptive_demux2_stream_load_a_fragment(stream=0x00000001330f1800) at gstadaptivedemux-stream.c:1969
 #5: 0x000000011271c2a4 libgstadaptivedemux2.dylib`gst_adaptive_demux2_stream_next_download(stream=0x00000001330f1800) at gstadaptivedemux-stream.c:2112
```

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5435>
This commit is contained in:
Thibault Saunier 2023-10-04 11:09:37 -03:00 committed by GStreamer Marge Bot
parent bdbf6e1c17
commit 049859c2cb
3 changed files with 24 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
subprojects/gst-plugins-good/ext/adaptivedemux2/downloadhelper.c
View file

@ -268,7 +268,7 @@ on_read_ready (GObject * source, GAsyncResult * result, gpointer user_data)
if (!g_cancellable_is_cancelled (transfer->cancellable)) { if (!g_cancellable_is_cancelled (transfer->cancellable)) {
GST_ERROR ("Failed to read stream: %s", error->message); GST_ERROR ("Failed to read stream: %s", error->message);
if (request->state != DOWNLOAD_REQUEST_STATE_UNSENT) if (request->state != DOWNLOAD_REQUEST_STATE_CANCELLED)
request->state = DOWNLOAD_REQUEST_STATE_ERROR; request->state = DOWNLOAD_REQUEST_STATE_ERROR;
finish_transfer_task (dh, transfer_task, error); finish_transfer_task (dh, transfer_task, error);
} else { } else {
@ -311,9 +311,8 @@ on_read_ready (GObject * source, GAsyncResult * result, gpointer user_data)
} }
if (gst_buffer != NULL) { if (gst_buffer != NULL) {
/* Unsent means cancellation is in progress, so don't override /* Don't override CANCELLED state. Otherwise make sure it is LOADING */
* the state. Otherwise make sure it is LOADING */ if (request->state != DOWNLOAD_REQUEST_STATE_CANCELLED)
if (request->state != DOWNLOAD_REQUEST_STATE_UNSENT)
request->state = DOWNLOAD_REQUEST_STATE_LOADING; request->state = DOWNLOAD_REQUEST_STATE_LOADING;
if (request->download_start_time == GST_CLOCK_TIME_NONE) { if (request->download_start_time == GST_CLOCK_TIME_NONE) {
@ -357,14 +356,15 @@ finish_transfer:
request->uri, request->range_start, request->range_end); request->uri, request->range_start, request->range_end);
#endif #endif
if (request->state != DOWNLOAD_REQUEST_STATE_UNSENT) { if (request->state != DOWNLOAD_REQUEST_STATE_CANCELLED) {
if (SOUP_STATUS_IS_SUCCESSFUL (status_code) if (SOUP_STATUS_IS_SUCCESSFUL (status_code)
|| SOUP_STATUS_IS_REDIRECTION (status_code)) || SOUP_STATUS_IS_REDIRECTION (status_code)) {
request->state = DOWNLOAD_REQUEST_STATE_COMPLETE; request->state = DOWNLOAD_REQUEST_STATE_COMPLETE;
else } else {
request->state = DOWNLOAD_REQUEST_STATE_ERROR; request->state = DOWNLOAD_REQUEST_STATE_ERROR;
} }
} }
}
request->download_end_time = now; request->download_end_time = now;
g_free (transfer->read_buffer); g_free (transfer->read_buffer);
@ -532,7 +532,7 @@ on_request_sent (GObject * source, GAsyncResult * result, gpointer user_data)
G_GINT64_FORMAT, request->status_code, request->uri, G_GINT64_FORMAT, request->status_code, request->uri,
request->range_start, request->range_end); request->range_start, request->range_end);
if (request->state != DOWNLOAD_REQUEST_STATE_UNSENT) if (request->state != DOWNLOAD_REQUEST_STATE_CANCELLED)
request->state = DOWNLOAD_REQUEST_STATE_ERROR; request->state = DOWNLOAD_REQUEST_STATE_ERROR;
finish_transfer_task (dh, transfer_task, error); finish_transfer_task (dh, transfer_task, error);
} else { } else {
@ -547,8 +547,8 @@ on_request_sent (GObject * source, GAsyncResult * result, gpointer user_data)
return; return;
} }
/* If the state went back to UNSENT, we were cancelled so don't override it */ /* If the state is cancelled don't override it */
if (request->state != DOWNLOAD_REQUEST_STATE_UNSENT && if (request->state != DOWNLOAD_REQUEST_STATE_CANCELLED &&
request->state != DOWNLOAD_REQUEST_STATE_HEADERS_RECEIVED) { request->state != DOWNLOAD_REQUEST_STATE_HEADERS_RECEIVED) {
request->state = DOWNLOAD_REQUEST_STATE_HEADERS_RECEIVED; request->state = DOWNLOAD_REQUEST_STATE_HEADERS_RECEIVED;
@ -589,7 +589,9 @@ finish_transfer_error:
GST_LOG ("request complete. Code %d URI %s range %" G_GINT64_FORMAT " %" GST_LOG ("request complete. Code %d URI %s range %" G_GINT64_FORMAT " %"
G_GINT64_FORMAT, _soup_message_get_status (msg), request->uri, G_GINT64_FORMAT, _soup_message_get_status (msg), request->uri,
request->range_start, request->range_end); request->range_start, request->range_end);
if (request->state != DOWNLOAD_REQUEST_STATE_UNSENT)
/* If the state is cancelled don't override it */
if (request->state != DOWNLOAD_REQUEST_STATE_CANCELLED)
request->state = DOWNLOAD_REQUEST_STATE_ERROR; request->state = DOWNLOAD_REQUEST_STATE_ERROR;
} }
@ -693,6 +695,13 @@ submit_transfer (DownloadHelper * dh, GTask * transfer_task)
DownloadRequest *request = transfer->request; DownloadRequest *request = transfer->request;
download_request_lock (request); download_request_lock (request);
if (request->state == DOWNLOAD_REQUEST_STATE_CANCELLED) {
download_request_unlock (request);
GST_DEBUG ("Don't submit already cancelled transfer");
return;
}
request->state = DOWNLOAD_REQUEST_STATE_OPEN; request->state = DOWNLOAD_REQUEST_STATE_OPEN;
request->download_request_time = request->download_request_time =
gst_adaptive_demux_clock_get_time (dh->clock); gst_adaptive_demux_clock_get_time (dh->clock);
@ -803,8 +812,7 @@ downloadhelper_stop (DownloadHelper * dh)
DownloadRequest *request = transfer->request; DownloadRequest *request = transfer->request;
download_request_lock (request); download_request_lock (request);
/* Reset the state to UNSENT, to indicate cancellation, like an XMLHttpRequest does */ request->state = DOWNLOAD_REQUEST_STATE_CANCELLED;
request->state = DOWNLOAD_REQUEST_STATE_UNSENT;
download_request_unlock (request); download_request_unlock (request);
transfer->complete = TRUE; transfer->complete = TRUE;
@ -970,8 +978,7 @@ downloadhelper_cancel_request (DownloadHelper * dh, DownloadRequest * request)
GST_DEBUG ("Cancelling request for URI %s range %" G_GINT64_FORMAT " %" GST_DEBUG ("Cancelling request for URI %s range %" G_GINT64_FORMAT " %"
G_GINT64_FORMAT, request->uri, request->range_start, request->range_end); G_GINT64_FORMAT, request->uri, request->range_start, request->range_end);
request->state = DOWNLOAD_REQUEST_STATE_UNSENT; request->state = DOWNLOAD_REQUEST_STATE_CANCELLED;
for (i = dh->active_transfers->len - 1; i >= 0; i--) { for (i = dh->active_transfers->len - 1; i >= 0; i--) {
GTask *transfer_task = g_array_index (dh->active_transfers, GTask *, i); GTask *transfer_task = g_array_index (dh->active_transfers, GTask *, i);
DownloadHelperTransfer *transfer = g_task_get_task_data (transfer_task); DownloadHelperTransfer *transfer = g_task_get_task_data (transfer_task);

1
subprojects/gst-plugins-good/ext/adaptivedemux2/downloadrequest.c
View file

@ -162,6 +162,7 @@ download_request_despatch_completion (DownloadRequest * request)
DownloadRequestPrivate *priv = DOWNLOAD_REQUEST_PRIVATE (request); DownloadRequestPrivate *priv = DOWNLOAD_REQUEST_PRIVATE (request);
switch (request->state) { switch (request->state) {
case DOWNLOAD_REQUEST_STATE_UNSENT: case DOWNLOAD_REQUEST_STATE_UNSENT:
case DOWNLOAD_REQUEST_STATE_CANCELLED:
if (priv->cancellation_cb) if (priv->cancellation_cb)
priv->cancellation_cb (request, request->state, priv->cb_data); priv->cancellation_cb (request, request->state, priv->cb_data);
break; break;

1
subprojects/gst-plugins-good/ext/adaptivedemux2/downloadrequest.h
View file

@ -41,6 +41,7 @@ enum _DownloadRequestState {
DOWNLOAD_REQUEST_STATE_LOADING, /* Content loading in progress */ DOWNLOAD_REQUEST_STATE_LOADING, /* Content loading in progress */
DOWNLOAD_REQUEST_STATE_COMPLETE, /* Request processing finished successfully - check status_code for completion 200-399 codes */ DOWNLOAD_REQUEST_STATE_COMPLETE, /* Request processing finished successfully - check status_code for completion 200-399 codes */
DOWNLOAD_REQUEST_STATE_ERROR, /* Request generated an http error - check status_code */ DOWNLOAD_REQUEST_STATE_ERROR, /* Request generated an http error - check status_code */
DOWNLOAD_REQUEST_STATE_CANCELLED, /* Request has been cancelled by the user */
}; };
struct _DownloadRequest struct _DownloadRequest