On 2020-08-19 04:11, Christian Schoenebeck wrote:
On Dienstag, 18. August 2020 14:40:36 CEST Geoffrey McRae wrote:
Due to a ridiculous commit in the Jack library, the client may have
been
freed already by a secondary audio device recovering its session.
https://github.com/jackaudio/jack2/issues/627
Until there is a proper fix for this we can not risk using the pointer
at all if we have been notified of a shutdown as it may have been
freed
by the jack library, as such the close call is commented out to
prevent
a use after free segfault.
At this time, this will not cause a memory leak as the recovery
routine
will trigger the "cleanup" code in the jack library, however, if this
is
ever corrected in the jack library this will need to be revisited.
Signed-off-by: Geoffrey McRae <ge...@hostfission.com>
---
audio/jackaudio.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/audio/jackaudio.c b/audio/jackaudio.c
index 72ed7c4929..e8faf1bb89 100644
--- a/audio/jackaudio.c
+++ b/audio/jackaudio.c
@@ -563,7 +563,22 @@ static void qjack_client_fini(QJackClient *c)
/* fallthrough */
case QJACK_STATE_SHUTDOWN:
- jack_client_close(c->client);
+ /*
+ * Due to a rediculous commit in the Jack library, the client
may
have + * been freed already.
No need to be offending, and especially no need to insult Stéphane in
QEMU
code.
Fair enough, I was not intending to offend Stéphane, and I apologize for
this.
I will revise this patch.
Could you please describe in more detail how you ran into this
situation with
your 2nd audio device?
Sure. Run a Windows guest with two audio devices, let it boot up, then
restart
the jack service to trigger the recovery routine, then attempt to use
the 2nd
(non-primary) audio device. Ie, go to windows audio settings to test the
microphone of the second audio device.
When windows try to use the 2nd audio device it goes through the
recovery
routine triggering this fault.
+ *
+ * Until there is a proper fix for this we can not risk using
the
+ * pointer at all if we have been notified of a shutdown, as
such
the + * below line is commented out to prevent a use after
free
segfault. + * This will not cause a memory leak as the
recovery
routine will trigger + * the "cleanup" code in the jack
library.
+ *
+ *
https://github.com/jackaudio/jack2/commit/171a3c4a0ddd18d2afae56f3af6291c8e
96ee3ac + */
+
+ //jack_client_close(c->client);
+ c->client = NULL;
+
/* fallthrough */
Are you aware that there are two distinct variants of JACK? They are
commonly
referred to as JACK1 vs. JACK2 and despite their names, they are in
fact
completely separate implementations and there are people who prefer one
over
the other. Your change would affect JACK1 as well.
I am aware and since these libraries are interchangeable I had assumed
that
JACK1 will have the same fault. If not I suppose we need to detect which
is in
use and change this code appropriately.
Best regards,
Christian Schoenebeck