ges: asset: Avoid trying to load twice the same asset

When requesting an asset from different threads we had no
guarantee that during the time we lookup an asset (which didn't exist)
and the time we create the asset with the same type/ID another thread
could not end up doing the same thing. In turns we could end up with
2 different threads loading the exact same asset and the cache
basically forgetting about one of the entries meaning that the user
would never get notified about one of those being ready to be used.

There was also the case when requesting "sync" where the user was
requesting an asset while another thread is creating it so it was
still in "ASSET_INITIALIZING" state, meaning that the returned asset
would be NULL which would be considered as an error in apps.

Since the cache lock is recursive we can just take it during the whole
ges_asset_request_async call and have other method still hold it.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5732>
This commit is contained in:
Thibault Saunier 2023-08-18 10:31:43 -04:00 committed by GStreamer Marge Bot
parent b405ff34d3
commit 96cf1f8c8e

View file

@ -1254,6 +1254,7 @@ ges_asset_request (GType extractable_type, const gchar * id, GError ** error)
g_error_free (lerr);
/* asset owned by cache */
LOCK_CACHE;
asset = ges_asset_cache_lookup (extractable_type, real_id);
if (asset) {
while (proxied) {
@ -1318,6 +1319,7 @@ ges_asset_request (GType extractable_type, const gchar * id, GError ** error)
}
g_type_class_unref (klass);
}
UNLOCK_CACHE;
if (real_id)
g_free (real_id);
@ -1403,7 +1405,11 @@ ges_asset_request_async (GType extractable_type,
}
/* Check if we already have an asset for this ID */
LOCK_CACHE;
asset = ges_asset_cache_lookup (extractable_type, real_id);
GESAssetCacheEntry *entry = _lookup_entry (extractable_type, id);
if (entry)
asset = entry->asset;
if (asset) {
task = g_task_new (asset, NULL, callback, user_data);
@ -1420,7 +1426,7 @@ ges_asset_request_async (GType extractable_type,
goto done;
case ASSET_INITIALIZING:
GST_DEBUG_OBJECT (asset, "Asset in cache and but not "
GST_DEBUG_OBJECT (asset, "Asset in cache but not "
"initialized, setting a new callback");
ges_asset_cache_append_task (extractable_type, real_id, task);
task = NULL;
@ -1455,6 +1461,7 @@ ges_asset_request_async (GType extractable_type,
goto done;
default:
GST_WARNING ("Case %i not handle, returning", asset->priv->state);
UNLOCK_CACHE;
return;
}
}
@ -1463,7 +1470,9 @@ ges_asset_request_async (GType extractable_type,
g_async_initable_new_async (ges_extractable_type_get_asset_type
(extractable_type), G_PRIORITY_DEFAULT, cancellable, callback, user_data,
"id", real_id, "extractable-type", extractable_type, NULL);
done:
UNLOCK_CACHE;
if (task)
gst_object_unref (task);
if (real_id)