From c513e679135f05a5906292aec3740f11604b9bcc6bbd6ae867f4bf77f940cf8e Mon Sep 17 00:00:00 2001 From: Antonio Larrosa Date: Thu, 30 May 2024 15:04:55 +0000 Subject: [PATCH] Accepting request 1176883 from home:XRevan86 OBS-URL: https://build.opensuse.org/request/show/1176883 OBS-URL: https://build.opensuse.org/package/show/multimedia:libs/wireplumber?expand=0&rev=80 --- ...ransition-fix-memleak-when-error-set.patch | 25 ++++ ...-ensure-single-completion-and-finish.patch | 131 ++++++++++++++++++ ...ing-return-after-aborting-transition.patch | 28 ++++ ...tate-stream-fix-using-default-volume.patch | 48 +++++++ wireplumber.changes | 10 ++ wireplumber.spec | 6 +- 6 files changed, 247 insertions(+), 1 deletion(-) create mode 100644 0004-transition-fix-memleak-when-error-set.patch create mode 100644 0005-transition-ensure-single-completion-and-finish.patch create mode 100644 0006-linking-return-after-aborting-transition.patch create mode 100644 0007-state-stream-fix-using-default-volume.patch diff --git a/0004-transition-fix-memleak-when-error-set.patch b/0004-transition-fix-memleak-when-error-set.patch new file mode 100644 index 0000000..98e0e99 --- /dev/null +++ b/0004-transition-fix-memleak-when-error-set.patch @@ -0,0 +1,25 @@ +From 1ddfbc532c87fb0ad18e128d574e5c3b72089416 Mon Sep 17 00:00:00 2001 +From: Barnabas Pocze +Date: Sat, 18 May 2024 00:46:58 +0200 +Subject: [PATCH 4/7] transition: fix memory leak when error is already set + +Fixes: 18377fbf829ff2 ("transition: don't allow _return_error() to be called recursively") +--- + lib/wp/transition.c | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/lib/wp/transition.c b/lib/wp/transition.c +index 547f555f0..0538208f4 100644 +--- a/lib/wp/transition.c ++++ b/lib/wp/transition.c +@@ -493,6 +493,7 @@ wp_transition_return_error (WpTransition * self, GError * error) + if (G_UNLIKELY (priv->error)) { + wp_warning_object (self, "transition bailing out multiple times; " + "new error is: %s", error->message); ++ g_error_free (error); + return; + } + +-- +2.45.1 + diff --git a/0005-transition-ensure-single-completion-and-finish.patch b/0005-transition-ensure-single-completion-and-finish.patch new file mode 100644 index 0000000..fa3ef65 --- /dev/null +++ b/0005-transition-ensure-single-completion-and-finish.patch @@ -0,0 +1,131 @@ +From 89b6766cd6a64c8d52512ae2c091de3f5aae034f Mon Sep 17 00:00:00 2001 +From: Barnabas Pocze +Date: Sat, 18 May 2024 00:53:33 +0200 +Subject: [PATCH 5/7] transition: ensure single completion and finish + +At the moment, the same transition can be completed multiple times +(assuming sufficiently high reference count): + + return_error() + finish() + return_error() + +The consequence of that is that the transition closure will be +invoked multiple times. This is unexpected. + +Add some guards against completing a transition or calling finish() +multiple times. + +Fixes #628 +--- + lib/wp/transition.c | 34 ++++++++++++++++++++++++++++------ + 1 file changed, 28 insertions(+), 6 deletions(-) + +diff --git a/lib/wp/transition.c b/lib/wp/transition.c +index 0538208f4..1e8bba2f5 100644 +--- a/lib/wp/transition.c ++++ b/lib/wp/transition.c +@@ -66,6 +66,8 @@ struct _WpTransitionPrivate + + /* state machine */ + gboolean started; ++ gboolean completed; ++ gboolean finished; + guint step; + GError *error; + }; +@@ -348,8 +350,7 @@ wp_transition_get_completed (WpTransition * self) + g_return_val_if_fail (WP_IS_TRANSITION (self), FALSE); + + WpTransitionPrivate *priv = wp_transition_get_instance_private (self); +- return (priv->step == WP_TRANSITION_STEP_NONE && priv->started) || +- priv->step == WP_TRANSITION_STEP_ERROR; ++ return priv->completed; + } + + /*! +@@ -370,6 +371,8 @@ wp_transition_had_error (WpTransition * self) + static void + wp_transition_return (WpTransition * self, WpTransitionPrivate *priv) + { ++ g_assert (priv->completed); ++ + if (priv->closure) { + GValue values[2] = { G_VALUE_INIT, G_VALUE_INIT }; + g_value_init (&values[0], G_TYPE_OBJECT); +@@ -427,6 +430,11 @@ wp_transition_advance (WpTransition * self) + guint next_step; + GError *error = NULL; + ++ if (priv->completed) { ++ wp_warning_object (priv->source_object, "tried to advance completed transition"); ++ return; ++ } ++ + priv->started = TRUE; + + if (g_cancellable_set_error_if_cancelled (priv->cancellable, &error)) { +@@ -442,17 +450,20 @@ wp_transition_advance (WpTransition * self) + + if (next_step == WP_TRANSITION_STEP_ERROR) { + /* return error if the callback didn't do it already */ +- if (G_UNLIKELY (!priv->error)) { ++ if (G_UNLIKELY (!priv->completed)) { + wp_transition_return_error (self, g_error_new (WP_DOMAIN_LIBRARY, + WP_LIBRARY_ERROR_INVARIANT, "state machine error")); + } + return; + } + ++ g_assert (!priv->completed); ++ + /* if we reached STEP_NONE again, that means we reached the next state */ + if (next_step == WP_TRANSITION_STEP_NONE) { + /* complete the transition */ + priv->step = next_step; ++ priv->completed = TRUE; + wp_transition_return (self, priv); + return; + } +@@ -490,15 +501,16 @@ wp_transition_return_error (WpTransition * self, GError * error) + + /* don't allow _return_error() to be called multiple times, + as it is dangerous to recurse in execute_step() */ +- if (G_UNLIKELY (priv->error)) { +- wp_warning_object (self, "transition bailing out multiple times; " +- "new error is: %s", error->message); ++ if (G_UNLIKELY (priv->completed)) { ++ wp_warning_object (priv->source_object, ++ "tried to set error on completed transition: %s", error->message); + g_error_free (error); + return; + } + + priv->step = WP_TRANSITION_STEP_ERROR; + priv->error = error; ++ priv->completed = TRUE; + + /* allow the implementation to rollback changes */ + if (WP_TRANSITION_GET_CLASS (self)->execute_step) +@@ -535,8 +547,18 @@ wp_transition_finish (GAsyncResult * res, GError ** error) + priv->step = WP_TRANSITION_STEP_ERROR; + g_propagate_error (error, g_error_new (WP_DOMAIN_LIBRARY, + WP_LIBRARY_ERROR_INVARIANT, "finished before starting")); ++ } else if (!priv->completed) { ++ priv->step = WP_TRANSITION_STEP_ERROR; ++ g_propagate_error (error, g_error_new (WP_DOMAIN_LIBRARY, ++ WP_LIBRARY_ERROR_INVARIANT, "finished before completion")); ++ } else if (priv->finished) { ++ priv->step = WP_TRANSITION_STEP_ERROR; ++ g_propagate_error (error, g_error_new (WP_DOMAIN_LIBRARY, ++ WP_LIBRARY_ERROR_INVARIANT, "finished multiple times")); + } + ++ priv->finished = TRUE; ++ + wp_trace_object (priv->source_object, "transition: finished %s", + (priv->step == WP_TRANSITION_STEP_NONE) ? "ok" : "with error"); + +-- +2.45.1 + diff --git a/0006-linking-return-after-aborting-transition.patch b/0006-linking-return-after-aborting-transition.patch new file mode 100644 index 0000000..02e9c0b --- /dev/null +++ b/0006-linking-return-after-aborting-transition.patch @@ -0,0 +1,28 @@ +From 4ed51791e03b63adbaf792564aa201a6d71a1050 Mon Sep 17 00:00:00 2001 +From: Barnabas Pocze +Date: Sat, 18 May 2024 01:04:42 +0200 +Subject: [PATCH 6/7] linking: return after aborting transition + +Avoid calling return_error() on the same transition multiple times. + +Fixes: 4b153ec55354da ("link-target.lua: change into a async hook") +See #628 +--- + src/scripts/linking/link-target.lua | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/src/scripts/linking/link-target.lua b/src/scripts/linking/link-target.lua +index 62be86cec..f49bf513f 100644 +--- a/src/scripts/linking/link-target.lua ++++ b/src/scripts/linking/link-target.lua +@@ -50,6 +50,7 @@ AsyncEventHook { + si_flags.failed_count > 5 then + transition:return_error ("tried to link on last rescan, not retrying " + .. tostring (si_link)) ++ return + end + + if si_props ["item.factory.name"] == "si-audio-virtual" then +-- +2.45.1 + diff --git a/0007-state-stream-fix-using-default-volume.patch b/0007-state-stream-fix-using-default-volume.patch new file mode 100644 index 0000000..06a7737 --- /dev/null +++ b/0007-state-stream-fix-using-default-volume.patch @@ -0,0 +1,48 @@ +From df8bc12464c6b582fc1a5f46e4a3a025557bc58b Mon Sep 17 00:00:00 2001 +From: George Kiagiadakis +Date: Fri, 24 May 2024 20:36:28 +0300 +Subject: [PATCH 7/7] state-stream: fix using the default volume + +When there was no previous state stored, the default volume was +also not applied because the code would return from the hook early + +Fixes: #655 +--- + src/scripts/node/state-stream.lua | 10 +++++----- + 1 file changed, 5 insertions(+), 5 deletions(-) + +diff --git a/src/scripts/node/state-stream.lua b/src/scripts/node/state-stream.lua +index f5ac95d54..e2fc7efee 100644 +--- a/src/scripts/node/state-stream.lua ++++ b/src/scripts/node/state-stream.lua +@@ -52,10 +52,7 @@ restore_stream_hook = SimpleEventHook { + return + end + +- local stored_values = getStoredStreamProps (key) +- if not stored_values then +- return +- end ++ local stored_values = getStoredStreamProps (key) or {} + + -- restore node Props (volumes, channelMap, etc...) + if Settings.get_boolean ("node.stream.restore-props") and stream_props ["state.restore-props"] ~= "false" +@@ -356,11 +353,14 @@ function buildDefaultChannelVolumes (node) + end + end + ++ log:info (node, "using default volume: " .. tostring(def_vol) .. ++ ", channels: " .. tostring(channels)) ++ + while (#res < channels) do + table.insert(res, def_vol) + end + +- return res; ++ return res + end + + function getStoredStreamProps (key) +-- +2.45.1 + diff --git a/wireplumber.changes b/wireplumber.changes index ba0ab5e..7ccbaaf 100644 --- a/wireplumber.changes +++ b/wireplumber.changes @@ -1,3 +1,13 @@ +------------------------------------------------------------------- +Sat May 25 13:28:01 UTC 2024 - Alexei Sorokin + +- Add patches from upstream to fix a crash for aborted links: + * 0004-transition-fix-memleak-when-error-set.patch + * 0005-transition-ensure-single-completion-and-finish.patch + * 0006-linking-return-after-aborting-transition.patch +- Add patch from upstream to fix default playback volume ignore: + * 0007-state-stream-fix-using-default-volume.patch + ------------------------------------------------------------------- Mon May 6 16:23:47 UTC 2024 - Antonio Larrosa diff --git a/wireplumber.spec b/wireplumber.spec index faa3d9d..90ad85c 100644 --- a/wireplumber.spec +++ b/wireplumber.spec @@ -1,7 +1,7 @@ # # spec file for package wireplumber # -# Copyright (c) 2022 SUSE LLC +# Copyright (c) 2024 SUSE LLC # # All modifications and additions to the file contributed by third parties # remain the property of their copyright owners, unless otherwise agreed @@ -33,6 +33,10 @@ Source1: split-config-file.py Patch0: 0001-lua-json-fix-error-ouput.patch Patch1: 0002-lua-json-add-method-to-merge-json-containers.patch Patch2: 0003-json-utils-fix-overriding-of-non-container-values-when.patch +Patch3: 0004-transition-fix-memleak-when-error-set.patch +Patch4: 0005-transition-ensure-single-completion-and-finish.patch +Patch5: 0006-linking-return-after-aborting-transition.patch +Patch6: 0007-state-stream-fix-using-default-volume.patch # docs BuildRequires: doxygen BuildRequires: graphviz