From 50c73d02f071b1dee542d7b665fbd87756c7c107 Mon Sep 17 00:00:00 2001
From: Zaggy1024 <zaggy1024@gmail.com>
Date: Sun, 6 Aug 2023 01:59:47 -0500
Subject: [PATCH] LibAudio: Add a test for creating and destructing a
 PlaybackStream

This will ensure that we don't leak any memory while playing back
audio.

There is an expectation value in the test that is only set to true when
PulseAudio is present for the moment. When any new implementation is
added for other libraries/platforms, we should hopefully get a CI
failure due to unexpected success in creating the `PlaybackStream`.

To ensure that we clean up our PulseAudio connection whenever audio
output is not needed, add `PulseAudioContext::weak_instance()` to allow
us to check whether an instance exists without creating one.
---
 Meta/Lagom/CMakeLists.txt                     |  5 +++
 Tests/LibAudio/CMakeLists.txt                 |  1 +
 Tests/LibAudio/TestPlaybackStream.cpp         | 44 +++++++++++++++++++
 .../Libraries/LibAudio/PulseAudioWrappers.cpp |  8 +++-
 .../Libraries/LibAudio/PulseAudioWrappers.h   |  1 +
 5 files changed, 58 insertions(+), 1 deletion(-)
 create mode 100644 Tests/LibAudio/TestPlaybackStream.cpp

diff --git a/Meta/Lagom/CMakeLists.txt b/Meta/Lagom/CMakeLists.txt
index 484420c759..42bfd9bf88 100644
--- a/Meta/Lagom/CMakeLists.txt
+++ b/Meta/Lagom/CMakeLists.txt
@@ -661,6 +661,11 @@ if (BUILD_LAGOM)
         # The FLAC tests need a special working directory to find the test files
         lagom_test(../../Tests/LibAudio/TestFLACSpec.cpp LIBS LibAudio WORKING_DIRECTORY "${FLAC_TEST_PATH}/..")
 
+        lagom_test(../../Tests/LibAudio/TestPlaybackStream.cpp LIBS LibAudio)
+        if (HAVE_PULSEAUDIO)
+            target_compile_definitions(TestPlaybackStream PRIVATE HAVE_PULSEAUDIO=1)
+        endif()
+
         # LibCore
         if ((LINUX OR APPLE) AND NOT EMSCRIPTEN)
             lagom_test(../../Tests/LibCore/TestLibCoreFileWatcher.cpp)
diff --git a/Tests/LibAudio/CMakeLists.txt b/Tests/LibAudio/CMakeLists.txt
index 605e855bd9..38ddf795d8 100644
--- a/Tests/LibAudio/CMakeLists.txt
+++ b/Tests/LibAudio/CMakeLists.txt
@@ -1,5 +1,6 @@
 set(TEST_SOURCES
     TestFLACSpec.cpp
+    TestPlaybackStream.cpp
 )
 
 foreach(source IN LISTS TEST_SOURCES)
diff --git a/Tests/LibAudio/TestPlaybackStream.cpp b/Tests/LibAudio/TestPlaybackStream.cpp
new file mode 100644
index 0000000000..246c22a87f
--- /dev/null
+++ b/Tests/LibAudio/TestPlaybackStream.cpp
@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2023, Gregory Bertilson <zaggy1024@gmail.com>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <AK/Math.h>
+#include <AK/MemoryStream.h>
+#include <AK/WeakPtr.h>
+#include <LibAudio/PlaybackStream.h>
+#include <LibTest/TestSuite.h>
+#include <unistd.h>
+
+#if defined(HAVE_PULSEAUDIO)
+#    include <LibAudio/PulseAudioWrappers.h>
+#endif
+
+TEST_CASE(create_and_destroy_playback_stream)
+{
+    bool has_implementation = false;
+#if defined(HAVE_PULSEAUDIO)
+    has_implementation = true;
+#endif
+
+    {
+        auto stream_result = Audio::PlaybackStream::create(Audio::OutputState::Playing, 44100, 2, 100, [](Bytes buffer, Audio::PcmSampleFormat format, size_t sample_count) -> ReadonlyBytes {
+            VERIFY(format == Audio::PcmSampleFormat::Float32);
+            FixedMemoryStream writing_stream { buffer };
+
+            for (size_t i = 0; i < sample_count; i++) {
+                MUST(writing_stream.write_value(0.0f));
+                MUST(writing_stream.write_value(0.0f));
+            }
+
+            return buffer.trim(writing_stream.offset());
+        });
+        EXPECT_EQ(!stream_result.is_error(), has_implementation);
+        usleep(10000);
+    }
+
+#if defined(HAVE_PULSEAUDIO)
+    VERIFY(!Audio::PulseAudioContext::weak_instance());
+#endif
+}
diff --git a/Userland/Libraries/LibAudio/PulseAudioWrappers.cpp b/Userland/Libraries/LibAudio/PulseAudioWrappers.cpp
index d8415fbf6f..1c51475716 100644
--- a/Userland/Libraries/LibAudio/PulseAudioWrappers.cpp
+++ b/Userland/Libraries/LibAudio/PulseAudioWrappers.cpp
@@ -11,10 +11,15 @@
 
 namespace Audio {
 
-ErrorOr<NonnullRefPtr<PulseAudioContext>> PulseAudioContext::instance()
+WeakPtr<PulseAudioContext> PulseAudioContext::weak_instance()
 {
     // Use a weak pointer to allow the context to be shut down if we stop outputting audio.
     static WeakPtr<PulseAudioContext> the_instance;
+    return the_instance;
+}
+
+ErrorOr<NonnullRefPtr<PulseAudioContext>> PulseAudioContext::instance()
+{
     static Threading::Mutex instantiation_mutex;
     // Lock and unlock the mutex to ensure that the mutex is fully unlocked at application
     // exit.
@@ -25,6 +30,7 @@ ErrorOr<NonnullRefPtr<PulseAudioContext>> PulseAudioContext::instance()
 
     auto instantiation_locker = Threading::MutexLocker(instantiation_mutex);
 
+    auto the_instance = weak_instance();
     RefPtr<PulseAudioContext> strong_instance_pointer = the_instance.strong_ref();
 
     if (strong_instance_pointer == nullptr) {
diff --git a/Userland/Libraries/LibAudio/PulseAudioWrappers.h b/Userland/Libraries/LibAudio/PulseAudioWrappers.h
index a40fcc431c..64e04f1eab 100644
--- a/Userland/Libraries/LibAudio/PulseAudioWrappers.h
+++ b/Userland/Libraries/LibAudio/PulseAudioWrappers.h
@@ -40,6 +40,7 @@ class PulseAudioContext
     : public AtomicRefCounted<PulseAudioContext>
     , public Weakable<PulseAudioContext> {
 public:
+    static AK::WeakPtr<PulseAudioContext> weak_instance();
     static ErrorOr<NonnullRefPtr<PulseAudioContext>> instance();
 
     explicit PulseAudioContext(pa_threaded_mainloop*, pa_mainloop_api*, pa_context*);