[Express:Bugfix] Guard null runtime in RuntimeManager::setCache#4617
Conversation
GitOrigin-RevId: e1d27551e71f9adf6c82a67479d91a3c4d65186d
|
|
| } | ||
| mInside->mCache.reset(new Cache); | ||
| mInside->mCache->cacheFile = cacheName; | ||
| mInside->mInfo->onSetCachePath(cacheName.c_str(), 0); |
There was a problem hiding this comment.
新增的空指针保护是合理的,可以避免在 runtime 未创建(如请求的后端不可用)时调用 mInside->mInfo->onSetCachePath 引发空指针解引用崩溃,符合 MNN 的错误处理与优雅降级要求。但有以下几点建议:
-
代码风格一致性:MNN 代码库中一般使用
mInside->mInfo == nullptr这种常规写法,Yoda 风格nullptr == mInside->mInfo在 MNN 中并不常见,建议改为常规写法以保持风格一致。 -
错误日志级别:此处属于可恢复的优雅降级场景,并非严重错误。可考虑使用
MNN_WARNING而非MNN_ERROR,避免给上层调用方造成误判。如果是 MNN 没有提供MNN_WARNING宏,则保持MNN_ERROR也可接受。 -
日志信息可读性:建议在日志中带上
cacheName,便于排查具体哪个 cache 设置被跳过,例如:MNN_ERROR("Runtime not created, skip setCache(%s)\n", cacheName.c_str());
-
状态后续一致性:当
mInfo为空时直接 return,mInside->mCache不会被创建。需确认后续逻辑(如updateCache、销毁流程)在mCache为空且mInfo为空时也能正常工作,不会出现其他路径上的悬空访问或断言失败。建议同时排查setCache之外对mInfo/mCache的访问路径是否都做了等价的保护。 -
线程安全:该检查位于
mLock临界区内,与后续使用mInfo之间无 TOCTOU 风险,这点没有问题。 -
设计层面建议(可选):从架构上看,出现
mInfo == nullptr通常是Runtime创建失败的延续状态。可考虑在Runtime创建失败时即记录一次明确的错误,避免后续每次调用(setCache、其他配置类接口)都需要重复判空与打日志;或者提供一个统一的isAvailable()接口让上层先判断。
GitOrigin-RevId: e1d27551e71f9adf6c82a67479d91a3c4d65186d
Description
Module
Type
Checklist
[Module:Type] Descriptionformat