Texture replacement: Fix DDS mip double free and VFS lifetime

- DDS files with mips stored level 0's file reference in level 1 too, so it
  was freed twice.
- The VFS was deleted on config changes (and ini reloads) while load tasks
  still used it. Now each cached texture waits for its task and releases its
  file references through the old VFS first, and reloads afterwards.
- A failed ini reload turns replacement off instead of leaving it on without
  a VFS.
- Release file references on purge.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Henrik RydgårdandClaude Opus 5.5 committed 2026-09-29 09:35:17 -06:00
1 parent 4df1b9c2c3
commit 45d997c3ea
4 files changed
+46 -16

No files matched your search

+28 -5
View File
@@ -105,13 +105,35 @@ ReplacedTexture::~ReplacedTexture() {
threadWaitable_ = nullptr;
}
ReleaseLevels();
}
void ReplacedTexture::ReleaseLevels() {
for (auto &level : levels_) {
// Null when replacement was switched off after we were cached - see NotifyConfigChanged.
if (vfs_) {
// vfs_ is null after Unload(), which has already released them.
if (vfs_ && level.fileRef) {
vfs_->ReleaseFile(level.fileRef);
}
level.fileRef = nullptr;
}
levels_.clear();
}
void ReplacedTexture::Unload() {
if (threadWaitable_) {
// The task holds its own copy of the VFS pointer, so it has to finish first.
threadWaitable_->WaitAndRelease();
threadWaitable_ = nullptr;
}
ReleaseLevels();
data_.clear();
fmt = Draw::DataFormat::UNDEFINED;
alphaStatus_ = TextureAlpha::Any;
if (State() != ReplacementState::UNLOADED) {
SetState(ReplacementState::UNLOADED);
}
vfs_ = nullptr;
}
void ReplacedTexture::PurgeIfNotUsedSinceTime(double t) {
@@ -138,7 +160,7 @@ void ReplacedTexture::PurgeIfNotUsedSinceTime(double t) {
}
data_.clear();
levels_.clear();
ReleaseLevels();
fmt = Draw::DataFormat::UNDEFINED;
alphaStatus_ = TextureAlpha::Any;
@@ -630,11 +652,12 @@ ReplacedTexture::LoadLevelResult ReplacedTexture::LoadLevelData(VFSFileReference
WARN_LOG(Log::TexReplacement, "DDS: Expected %d bytes, got %d", bytesToRead, (int)read_bytes);
}
if (i != 0) {
level.fileRef = nullptr; // We only provide a fileref on level 0 if we have mipmaps.
}
levels_.push_back(level);
level.w = std::max(level.w / 2, 1);
level.h = std::max(level.h / 2, 1);
if (i != 0)
level.fileRef = nullptr; // We only provide a fileref on level 0 if we have mipmaps.
}
vfs_->CloseFile(openFile);
+4
View File
@@ -215,6 +215,10 @@ private:
};
void Prepare(VFSBackend *vfs);
// Waits for any load task, then drops all data and file references, and detaches from the VFS.
// The texture reloads from scratch once it gets a VFS again. Must run before the VFS is deleted.
void Unload();
void ReleaseLevels();
LoadLevelResult LoadLevelData(VFSFileReference *fileRef, const std::string &filename, int level, Draw::DataFormat *pixelFormat);
void PurgeIfNotUsedSinceTime(double t);
+13 -11
View File
@@ -109,14 +109,7 @@ void TextureReplacer::NotifyConfigChanged() {
}
if (!replaceEnabled_ && wasReplaceEnabled) {
// Everything in levelCache_ holds this pointer - LoadIni fixes them up when it swaps the
// VFS, and the same has to happen here or they're left dangling. Decimate(ALL) below only
// frees their data, it doesn't erase the entries.
for (auto &repl : levelCache_) {
repl.second->vfs_ = nullptr;
}
delete vfs_;
vfs_ = nullptr;
DeleteVFS();
Decimate(ReplacerDecimateMode::ALL);
} else if (!wasReplaceEnabled && replaceEnabled_) {
std::string error;
@@ -131,13 +124,23 @@ void TextureReplacer::NotifyConfigChanged() {
std::string error;
bool result = LoadIni(&error, false);
if (!result) {
// Ignore errors here, just log if we successfully loaded an ini.
// Ignore errors here, just log if we successfully loaded an ini. But the VFS is gone, so replacement is off.
replaceEnabled_ = false;
} else {
INFO_LOG(Log::G3D, "Loaded INI file for saving.");
}
}
}
void TextureReplacer::DeleteVFS() {
// Everything in levelCache_ uses the VFS, and may have a load task running against it.
for (auto &repl : levelCache_) {
repl.second->Unload();
}
delete vfs_;
vfs_ = nullptr;
}
bool TextureReplacer::LoadIni(std::string *error, bool notify) {
textureHash_ = ReplacedTextureHash::QUICK;
aliases_.clear();
@@ -151,8 +154,7 @@ bool TextureReplacer::LoadIni(std::string *error, bool notify) {
// Prevents dumping the mipmaps.
ignoreMipmap_ = false;
delete vfs_;
vfs_ = nullptr;
DeleteVFS();
Path zipPath = basePath_ / ZIP_FILENAME;
+1
View File
@@ -116,6 +116,7 @@ protected:
bool FindFiltering(ReplacementCacheKey key, TextureFiltering *forceFiltering);
bool LoadIni(std::string *error, bool notify = true);
void DeleteVFS();
bool LoadIniValues(IniFile &ini, VFSBackend *dir, bool isOverride, std::string *error);
void ParseHashRange(const std::string &key, const std::string &value);
void ParseFiltering(const std::string &key, const std::string &value);