diff --git a/CHANGELOG.md b/CHANGELOG.md index a17a6c718..4b6c8df38 100644 --- 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 index 7c6d13086..952b61465 100644 --- 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(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 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 index 6bd20ea37..5ac7fafe5 100644 --- a/src/messages/Image.hpp +++ b/src/messages/Image.hpp @@ -19,6 +19,14 @@ #include #include +#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 @@ -105,6 +113,8 @@ private: // forward-declarable function that calls Image::getEmpty() under the hood. ImagePtr getEmptyImagePtr(); +#ifndef DISABLE_IMAGE_EXPIRATION_POOL + class ImageExpirationPool { private: @@ -131,4 +141,6 @@ private: std::mutex mutex_; }; +#endif + } // namespace chatterino