Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

修复cache.cfg丢时缓存映射错误 #2527

Merged
merged 7 commits into from
Oct 14, 2024
Merged

Conversation

fanmingyi
Copy link
Contributor

问题描述

近期我司产品有用户反馈发送的表情和预期不符.如发送一个哭的表情变为笑.

说明

缺陷 1

假设缓存关系如下:
http://hello.pag -> 1.bin
http://world.pag -> 2.bin
cache.cfg 被回收删除但是缓存文件依旧存在时,我们重启 app .我们调用readFile函数,传入 http://world.pag .

std::shared_ptr<tgfx::Data> DiskCache::readFile(const std::string& key) {
  std::lock_guard<std::mutex> autoLock(locker);
  if (cacheFolder.empty() || key.empty()) {
    return nullptr;
  }
  //由于缓存配置文件丢失这里重新从 1 开始自增,所以http://world.pag  返回 fileID 为 1
  auto fileID = getFileID(key);
  //重新映射文件路径.得到/sdcard/android/data/org.libpag.pagviewer/cache/files/1.bin
  auto filePath = fileIDToPath(fileID);
  //由于 1.bin 存储实际为http://hello.pag 对象.导致读取错误
  auto stream = tgfx::Stream::MakeFromFile(filePath);
  if (stream == nullptr) {
    return nullptr;
  }
  tgfx::Buffer buffer(stream->size());
  auto readLength = stream->read(buffer.data(), buffer.size());
  if (readLength != buffer.size()) {
    return nullptr;
  }
  return buffer.release();
}

缺陷 2

调用saveConfig 时被强制中断,会导致cache.cfg 丢失.从而导致缓存错误

void DiskCache::saveConfig() {
  Directory::CreateRecursively(Directory::GetParentDirectory(configPath));
  //wb 写入方式会重写文件.如果这时候文件读写中断会导致配置丢失
  auto file = fopen(configPath.c_str(), "wb");
//....
}

问题模拟复现

插入以下代码,在休眠期间强制停止

void DiskCache::saveConfig() {
    LOGE("fmy invoke saveConfig");
  Directory::CreateRecursively(Directory::GetParentDirectory(configPath));
  auto file = fopen(configPath.c_str(), "wb");
  if (file == nullptr) {
      LOGE("fmy invoke saveConfig file is nullptr");
    return;
  }
    LOGE("fmy invoke saveConfig start sleep");
    //模拟高耗时操作,然后在休眠期间 Android app 强制停止
    std::this_thread::sleep_for(std::chrono::seconds(15));
    LOGE("fmy invoke saveConfig end sleep");
    size_t bufferSize = 0;
  //...略
}

@codecov-commenter
Copy link

codecov-commenter commented Oct 14, 2024

Codecov Report

Attention: Patch coverage is 28.57143% with 5 lines in your changes missing coverage. Please review.

Project coverage is 77.33%. Comparing base (c77c593) to head (d345a1c).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
src/rendering/caches/DiskCache.cpp 28.57% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2527   +/-   ##
=======================================
  Coverage   77.32%   77.33%           
=======================================
  Files         420      420           
  Lines       22457    22460    +3     
  Branches     6325     6346   +21     
=======================================
+ Hits        17365    17369    +4     
- Misses       3818     3819    +1     
+ Partials     1274     1272    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This reverts commit e2b8d28.
@@ -73,6 +73,7 @@ DiskCache::DiskCache() {
if (!cacheDir.empty()) {
configPath = Directory::JoinPath(cacheDir, "cache.cfg");
cacheFolder = Directory::JoinPath(cacheDir, "files");
removeRedundancyCache();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removeRedundancyCache存在很多 readConfig里已经写过的重复代码,可以给 readConfig() 加个返回值,如果读取配置失败(不存在文件或者文件size为0)就返回false:然后判断返回值即可:
if (!readConfig()) { Directory::VisitFiles(cacheFolder, [&](const std::string& path, size_t) { remove(path.c_str()); }); }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

确实这样更优雅我改改

@@ -73,7 +73,11 @@ DiskCache::DiskCache() {
if (!cacheDir.empty()) {
configPath = Directory::JoinPath(cacheDir, "cache.cfg");
cacheFolder = Directory::JoinPath(cacheDir, "files");
readConfig();
auto ret = readConfig();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

变量命名要避免使用缩写ret,可以直接用if(!readConfig()),或者起个有明确含义的变量名

@domchen domchen merged commit 90899f5 into Tencent:main Oct 14, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants