Disable ImageExpirationPool during testing (#4363)

* Disable ImageExpirationPool during testing

* Update CHANGELOG.md

---------

Co-authored-by: pajlada <rasmus.karlsson@pajlada.com>
This commit is contained in:
Daniel Sage 2023-02-11 14:20:46 -05:00 committed by GitHub
parent 5179567334
commit cf80ae8434
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 29 additions and 8 deletions

View file

@ -47,6 +47,7 @@
- Dev: Added CMake Install Support on Windows. (#4300) - 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: 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: 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: 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) - Dev: Use `QEnterEvent` for `QWidget::enterEvent` on Qt 6. (#4365)

View file

@ -272,7 +272,9 @@ namespace detail {
// IMAGE2 // IMAGE2
Image::~Image() Image::~Image()
{ {
#ifndef DISABLE_IMAGE_EXPIRATION_POOL
ImageExpirationPool::instance().removeImagePtr(this); ImageExpirationPool::instance().removeImagePtr(this);
#endif
if (this->empty_ && !this->frames_) if (this->empty_ && !this->frames_)
{ {
@ -425,7 +427,9 @@ void Image::load() const
Image *this2 = const_cast<Image *>(this); Image *this2 = const_cast<Image *>(this);
this2->shouldLoad_ = false; this2->shouldLoad_ = false;
this2->actuallyLoad(); this2->actuallyLoad();
#ifndef DISABLE_IMAGE_EXPIRATION_POOL
ImageExpirationPool::instance().addImagePtr(this2->shared_from_this()); ImageExpirationPool::instance().addImagePtr(this2->shared_from_this());
#endif
} }
} }
@ -551,6 +555,8 @@ void Image::expireFrames()
this->shouldLoad_ = true; // Mark as needing load again this->shouldLoad_ = true; // Mark as needing load again
} }
#ifndef DISABLE_IMAGE_EXPIRATION_POOL
ImageExpirationPool::ImageExpirationPool() ImageExpirationPool::ImageExpirationPool()
{ {
QObject::connect(&this->freeTimer_, &QTimer::timeout, [this] { QObject::connect(&this->freeTimer_, &QTimer::timeout, [this] {
@ -593,10 +599,10 @@ void ImageExpirationPool::freeOld()
{ {
std::lock_guard<std::mutex> lock(this->mutex_); std::lock_guard<std::mutex> lock(this->mutex_);
#ifndef NDEBUG # ifndef NDEBUG
size_t numExpired = 0; size_t numExpired = 0;
size_t eligible = 0; size_t eligible = 0;
#endif # endif
auto now = std::chrono::steady_clock::now(); auto now = std::chrono::steady_clock::now();
for (auto it = this->allImages_.begin(); it != this->allImages_.end();) for (auto it = this->allImages_.begin(); it != this->allImages_.end();)
@ -617,17 +623,17 @@ void ImageExpirationPool::freeOld()
continue; continue;
} }
#ifndef NDEBUG # ifndef NDEBUG
++eligible; ++eligible;
#endif # endif
// Check if image has expired and, if so, expire its frame data // Check if image has expired and, if so, expire its frame data
auto diff = now - img->lastUsed_; auto diff = now - img->lastUsed_;
if (diff > IMAGE_POOL_IMAGE_LIFETIME) if (diff > IMAGE_POOL_IMAGE_LIFETIME)
{ {
#ifndef NDEBUG # ifndef NDEBUG
++numExpired; ++numExpired;
#endif # endif
img->expireFrames(); img->expireFrames();
// erase without mutex locking issue // erase without mutex locking issue
it = this->allImages_.erase(it); it = this->allImages_.erase(it);
@ -637,10 +643,12 @@ void ImageExpirationPool::freeOld()
++it; ++it;
} }
#ifndef NDEBUG # ifndef NDEBUG
qCDebug(chatterinoImage) << "freed frame data for" << numExpired << "/" qCDebug(chatterinoImage) << "freed frame data for" << numExpired << "/"
<< eligible << "eligible images"; << eligible << "eligible images";
#endif # endif
} }
#endif
} // namespace chatterino } // namespace chatterino

View file

@ -19,6 +19,14 @@
#include <memory> #include <memory>
#include <mutex> #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 chatterino {
namespace detail { namespace detail {
template <typename Image> template <typename Image>
@ -105,6 +113,8 @@ private:
// forward-declarable function that calls Image::getEmpty() under the hood. // forward-declarable function that calls Image::getEmpty() under the hood.
ImagePtr getEmptyImagePtr(); ImagePtr getEmptyImagePtr();
#ifndef DISABLE_IMAGE_EXPIRATION_POOL
class ImageExpirationPool class ImageExpirationPool
{ {
private: private:
@ -131,4 +141,6 @@ private:
std::mutex mutex_; std::mutex mutex_;
}; };
#endif
} // namespace chatterino } // namespace chatterino