Disable ImageExpirationPool during testing (Chatterino#4363)

* Disable ImageExpirationPool during testing * Update CHANGELOG.md --------- Co-authored-by: pajlada <[email protected]>
LosFarmosCTL · Feb 11, 2023 · cf80ae8 · cf80ae8
1 parent 5179567
commit cf80ae8
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -47,6 +47,7 @@
 - Dev: Added CMake Install Support on Windows. (#4300)
 - Dev: Changed conan generator to [`CMakeDeps`](https://docs.conan.io/en/latest/reference/conanfile/tools/cmake/cmakedeps.html) and [`CMakeToolchain`](https://docs.conan.io/en/latest/reference/conanfile/tools/cmake/cmaketoolchain.html). See PR for migration notes. (#4335)
 - Dev: Refactored 7TV EventAPI implementation. (#4342)
+- Dev: Disabled ImageExpirationPool in tests. (#4363)
 - Dev: Don't rely on undocumented registry keys to find the default browser on Windows. (#4362)
 - Dev: Use `QEnterEvent` for `QWidget::enterEvent` on Qt 6. (#4365)
 

diff --git a/src/messages/Image.cpp b/src/messages/Image.cpp
@@ -272,7 +272,9 @@ namespace detail {
 // IMAGE2
 Image::~Image()
 {
+#ifndef DISABLE_IMAGE_EXPIRATION_POOL
     ImageExpirationPool::instance().removeImagePtr(this);
+#endif
 
     if (this->empty_ && !this->frames_)
     {
@@ -425,7 +427,9 @@ void Image::load() const
         Image *this2 = const_cast<Image *>(this);
         this2->shouldLoad_ = false;
         this2->actuallyLoad();
+#ifndef DISABLE_IMAGE_EXPIRATION_POOL
         ImageExpirationPool::instance().addImagePtr(this2->shared_from_this());
+#endif
     }
 }
 
@@ -551,6 +555,8 @@ void Image::expireFrames()
     this->shouldLoad_ = true;  // Mark as needing load again
 }
 
+#ifndef DISABLE_IMAGE_EXPIRATION_POOL
+
 ImageExpirationPool::ImageExpirationPool()
 {
     QObject::connect(&this->freeTimer_, &QTimer::timeout, [this] {
@@ -593,10 +599,10 @@ void ImageExpirationPool::freeOld()
 {
     std::lock_guard<std::mutex> lock(this->mutex_);
 
-#ifndef NDEBUG
+#    ifndef NDEBUG
     size_t numExpired = 0;
     size_t eligible = 0;
-#endif
+#    endif
 
     auto now = std::chrono::steady_clock::now();
     for (auto it = this->allImages_.begin(); it != this->allImages_.end();)
@@ -617,17 +623,17 @@ void ImageExpirationPool::freeOld()
             continue;
         }
 
-#ifndef NDEBUG
+#    ifndef NDEBUG
         ++eligible;
-#endif
+#    endif
 
         // Check if image has expired and, if so, expire its frame data
         auto diff = now - img->lastUsed_;
         if (diff > IMAGE_POOL_IMAGE_LIFETIME)
         {
-#ifndef NDEBUG
+#    ifndef NDEBUG
             ++numExpired;
-#endif
+#    endif
             img->expireFrames();
             // erase without mutex locking issue
             it = this->allImages_.erase(it);
@@ -637,10 +643,12 @@ void ImageExpirationPool::freeOld()
         ++it;
     }
 
-#ifndef NDEBUG
+#    ifndef NDEBUG
     qCDebug(chatterinoImage) << "freed frame data for" << numExpired << "/"
                              << eligible << "eligible images";
-#endif
+#    endif
 }
 
+#endif
+
 }  // namespace chatterino
diff --git a/src/messages/Image.hpp b/src/messages/Image.hpp
@@ -19,6 +19,14 @@
 #include <memory>
 #include <mutex>
 
+#ifdef CHATTERINO_TEST
+// When running tests, the ImageExpirationPool destructor can be called before
+// all images are deleted, leading to a use-after-free of its mutex. This
+// happens despite the lifetime of the ImageExpirationPool being (apparently)
+// static. Therefore, just disable it during testing.
+#    define DISABLE_IMAGE_EXPIRATION_POOL
+#endif
+
 namespace chatterino {
 namespace detail {
     template <typename Image>
@@ -105,6 +113,8 @@ class Image : public std::enable_shared_from_this<Image>, boost::noncopyable
 // forward-declarable function that calls Image::getEmpty() under the hood.
 ImagePtr getEmptyImagePtr();
 
+#ifndef DISABLE_IMAGE_EXPIRATION_POOL
+
 class ImageExpirationPool
 {
 private:
@@ -131,4 +141,6 @@ class ImageExpirationPool
     std::mutex mutex_;
 };
 
+#endif
+
 }  // namespace chatterino