已合并
[render_service] Add null and error code checks for stability and remove deprecated API #31394
[render_service] Add null and error code checks for stability and remove deprecated API #31394
已合并
sunriseLL创建于 7月20日
23 个文件变更+148-90
@@ -2,3 +2,5 @@
2/adapter/ios2/adapter/ios
3/.vscode3/.vscode
4gitcode_mcp.log4gitcode_mcp.log
5+opencode.md
6+/.opencode
@@ -183,11 +183,6 @@ bool RSSystemProperties::GetCacheEnabledForRotation()
183 return {};183 return {};
184}184}
185 185 
186-ParallelRenderingType RSSystemProperties::GetPrepareParallelRenderingEnabled()
187-{
188- return {};
189-}
190- 
191ParallelRenderingType RSSystemProperties::GetParallelRenderingEnabled()186ParallelRenderingType RSSystemProperties::GetParallelRenderingEnabled()
192{187{
193 return {};188 return {};
@@ -705,7 +705,10 @@ void RSRenderNodeDrawable::InitDfxForCacheInfo()
705 705 
706#ifdef DDGR_ENABLE_FEATURE_OPINC706#ifdef DDGR_ENABLE_FEATURE_OPINC
707 autoCacheDrawingEnable_ = RSSystemProperties::GetAutoCacheDebugEnabled() && RSOpincDrawCache::IsAutoCacheEnable();707 autoCacheDrawingEnable_ = RSSystemProperties::GetAutoCacheDebugEnabled() && RSOpincDrawCache::IsAutoCacheEnable();
708- autoCacheRenderNodeInfos_.clear();708+ {
709+ std::lock_guard<std::mutex> lock(drawingCacheInfoMutex_);
710+ autoCacheRenderNodeInfos_.clear();
711+ }
chuchengcheng
chuchengchengchuchengcheng7月23日

🤖 AI 代码检视意见 | 🛡️ Reliability | ℹ️ Medium | 行号区间: L708-L711

问题描述: InitDfxForCacheInfo、DrawDfxForCacheInfo、DrawCachedImage 三处新增 std::lock_guardstd::mutex lock(drawingCacheInfoMutex_) 使用,但 diff 中未见 rs_render_node_drawable.h 的对应修改。若 drawingCacheInfoMutex_ 为新增成员而非既有成员,缺少头文件声明将导致编译失败;若为既有成员,说明此前该保护缺失属已存在缺陷。无论哪种情况,均需在头文件中确认声明存在且访问语义一致。

💡 查看修复建议(点击展开)

修改建议:确认 drawingCacheInfoMutex_ 已在 rs_render_node_drawable.h 中声明为 mutable std::mutex;若为新增,需同步提交头文件变更。

std::lock_guard<std::mutex> lock(drawingCacheInfoMutex_);
autoCacheRenderNodeInfos_.clear();
}
likedislike
709 ClearOpincState();712 ClearOpincState();
710#endif713#endif
711}714}
@@ -725,7 +728,12 @@ void RSRenderNodeDrawable::DrawDfxForCacheInfo(
725 }728 }
726 729 
727 if (autoCacheDrawingEnable_ && !isDrawingCacheDfxEnabled_) {730 if (autoCacheDrawingEnable_ && !isDrawingCacheDfxEnabled_) {
728- for (const auto& info : autoCacheRenderNodeInfos_) {731+ decltype(autoCacheRenderNodeInfos_) infosCopy;
732+ {
733+ std::lock_guard<std::mutex> lock(drawingCacheInfoMutex_);
chuchengchengsunriseLL
chuchengchengchuchengcheng7月23日

这些地方存在多线程的可能吗?

likedislike
sunriseLL
sunriseLL
7月23日 评论:
sunriseLLsunriseLL7月23日

已确认 subthread 可能并发访问 autoCacheRenderNodeInfos_,已在 InitDfxForCacheInfo、DrawDfxForCacheInfo、DrawCachedImage 三处加 drawingCacheInfoMutex_ 保护。DrawDfxForCacheInfo 读路径用 copy-under-lock 模式避免持锁做 canvas 绘制。

likedislike
734+ infosCopy = autoCacheRenderNodeInfos_;
735+ }
736+ for (const auto& info : infosCopy) {
729 RSUniRenderUtil::DrawRectForDfx(737 RSUniRenderUtil::DrawRectForDfx(
730 canvas, info.first, Drawing::Color::COLOR_BLUE, 0.2f, info.second); // alpha 0.2 by default738 canvas, info.first, Drawing::Color::COLOR_BLUE, 0.2f, info.second); // alpha 0.2 by default
731 }739 }
@@ -1061,8 +1069,13 @@ void RSRenderNodeDrawable::DrawCachedImage(
1061 Drawing::Brush brush;1069 Drawing::Brush brush;
1062 canvas.AttachBrush(brush);1070 canvas.AttachBrush(brush);
1063 auto samplingOptions = Drawing::SamplingOptions(Drawing::FilterMode::LINEAR, Drawing::MipmapMode::NONE);1071 auto samplingOptions = Drawing::SamplingOptions(Drawing::FilterMode::LINEAR, Drawing::MipmapMode::NONE);
1064- if (RSOpincDrawCacheHelper::TryDrawOpincAutoCache(*this, canvas, *cacheImage,1072+ bool opincCacheResult = false;
1065- samplingOptions, autoCacheRenderNodeInfos_)) {1073+ {
1074+ std::lock_guard<std::mutex> lock(drawingCacheInfoMutex_);
1075+ opincCacheResult = RSOpincDrawCacheHelper::TryDrawOpincAutoCache(*this, canvas, *cacheImage,
1076+ samplingOptions, autoCacheRenderNodeInfos_);
1077+ }
1078+ if (opincCacheResult) {
1066 canvas.DetachBrush();1079 canvas.DetachBrush();
1067 return;1080 return;
1068 }1081 }
@@ -220,7 +220,6 @@ public:
220 static bool GetDrawTextAsBitmap();220 static bool GetDrawTextAsBitmap();
221 static void SetCacheEnabledForRotation(bool flag);221 static void SetCacheEnabledForRotation(bool flag);
222 static bool GetCacheEnabledForRotation();222 static bool GetCacheEnabledForRotation();
223- static ParallelRenderingType GetPrepareParallelRenderingEnabled();
224 static ParallelRenderingType GetParallelRenderingEnabled();223 static ParallelRenderingType GetParallelRenderingEnabled();
225 static HgmRefreshRates GetHgmRefreshRatesEnabled();224 static HgmRefreshRates GetHgmRefreshRatesEnabled();
226 static void SetHgmRefreshRateModesEnabled(std::string param);225 static void SetHgmRefreshRateModesEnabled(std::string param);
@@ -16,6 +16,8 @@
16#ifndef RENDER_SERVICE_BASE_TRANSACTION_RS_MARSHALLING_HELPER_H16#ifndef RENDER_SERVICE_BASE_TRANSACTION_RS_MARSHALLING_HELPER_H
17#define RENDER_SERVICE_BASE_TRANSACTION_RS_MARSHALLING_HELPER_H17#define RENDER_SERVICE_BASE_TRANSACTION_RS_MARSHALLING_HELPER_H
18 18 
19+#include <securec.h>
20+ 
19#include <map>21#include <map>
20#include <memory>22#include <memory>
21#include <optional>23#include <optional>
@@ -147,7 +149,9 @@ public:
147 if (buff == nullptr) {149 if (buff == nullptr) {
148 return false;150 return false;
149 }151 }
150- val = *(reinterpret_cast<const T*>(buff));152+ if (memcpy_s(&val, sizeof(T), buff, sizeof(T)) != EOK) {
153+ return false;
154+ }
151 return true;155 return true;
152 }156 }
153 return false;157 return false;
@@ -348,8 +352,12 @@ public:
348 352 
349 // reloaded marshalling & unmarshalling function for std::map353 // reloaded marshalling & unmarshalling function for std::map
350 template<typename T, typename P>354 template<typename T, typename P>
351- static bool Marshalling(Parcel& parcel, const std::map<T, P>& val)355+ static bool Marshalling(Parcel& parcel, const std::map<T, P>& val,
356+ size_t maxSize = UNMARSHALLING_MAX_VECTOR_SIZE)
352 {357 {
358+ if (val.size() > maxSize) {
359+ return false;
360+ }
353 if (!parcel.WriteUint32(val.size())) {361 if (!parcel.WriteUint32(val.size())) {
354 return false;362 return false;
355 }363 }
@@ -362,12 +370,16 @@ public:
362 }370 }
363 371 
364 template<typename T, typename P>372 template<typename T, typename P>
365- static bool Unmarshalling(Parcel& parcel, std::map<T, P>& val)373+ static bool Unmarshalling(Parcel& parcel, std::map<T, P>& val,
374+ size_t maxSize = UNMARSHALLING_MAX_VECTOR_SIZE)
366 {375 {
367 uint32_t size = 0;376 uint32_t size = 0;
368 if (!Unmarshalling(parcel, size)) {377 if (!Unmarshalling(parcel, size)) {
369 return false;378 return false;
370 }379 }
380+ if (size > maxSize) {
381+ return false;
382+ }
371 val.clear();383 val.clear();
372 for (uint32_t i = 0; i < size; ++i) {384 for (uint32_t i = 0; i < size; ++i) {
373 T key;385 T key;
@@ -72,6 +72,7 @@ public:
72 void CloseSyncTransaction();72 void CloseSyncTransaction();
73 void SetFlushEmptyCallback(FlushEmptyCallback flushEmptyCallback)73 void SetFlushEmptyCallback(FlushEmptyCallback flushEmptyCallback)
74 {74 {
75+ std::lock_guard<std::mutex> lock(mutex_);
75 flushEmptyCallback_ = flushEmptyCallback;76 flushEmptyCallback_ = flushEmptyCallback;
76 }77 }
77 78 
@@ -62,6 +62,7 @@ public:
62 void CloseSyncTransaction();62 void CloseSyncTransaction();
63 void SetFlushEmptyCallback(FlushEmptyCallback flushEmptyCallback)63 void SetFlushEmptyCallback(FlushEmptyCallback flushEmptyCallback)
64 {64 {
65+ std::lock_guard<std::mutex> lock(mutex_);
65 flushEmptyCallback_ = flushEmptyCallback;66 flushEmptyCallback_ = flushEmptyCallback;
66 }67 }
67 68 
@@ -81,6 +82,7 @@ public:
81 82 
82 void SetRSRenderPipelineClient(std::shared_ptr<RSRenderPipelineClient> rsRenderPipelineClient)83 void SetRSRenderPipelineClient(std::shared_ptr<RSRenderPipelineClient> rsRenderPipelineClient)
83 {84 {
85+ std::scoped_lock lock(mutex_, mutexForRT_);
84 renderPipelineClient_ = rsRenderPipelineClient;86 renderPipelineClient_ = rsRenderPipelineClient;
85 }87 }
86 88 
@@ -218,6 +218,10 @@ void SurfaceNodeCommandHelper::MarkUIHidden(RSContext& context, NodeId id, bool
218 218 
219void SurfaceNodeCommandHelper::SetSurfaceNodeType(RSContext& context, NodeId nodeId, uint8_t surfaceNodeType)219void SurfaceNodeCommandHelper::SetSurfaceNodeType(RSContext& context, NodeId nodeId, uint8_t surfaceNodeType)
220{220{
221+ if (surfaceNodeType >= static_cast<uint8_t>(RSSurfaceNodeType::NODE_MAX)) {
222+ ROSEN_LOGE("SetSurfaceNodeType invalid type:%{public}u", surfaceNodeType);
223+ return;
224+ }
221 auto type = static_cast<RSSurfaceNodeType>(surfaceNodeType);225 auto type = static_cast<RSSurfaceNodeType>(surfaceNodeType);
222 if (auto node = context.GetNodeMap().GetRenderNode<RSSurfaceRenderNode>(nodeId)) {226 if (auto node = context.GetNodeMap().GetRenderNode<RSSurfaceRenderNode>(nodeId)) {
223 node->SetSurfaceNodeType(type);227 node->SetSurfaceNodeType(type);
@@ -1241,11 +1241,15 @@ void RSRenderNode::ChildrenListDump(std::string& out) const
1241 auto sortedChildren = GetSortedChildren();1241 auto sortedChildren = GetSortedChildren();
1242 const int childrenCntLimit = 10;1242 const int childrenCntLimit = 10;
1243 if (!isFullChildrenListValid_) {1243 if (!isFullChildrenListValid_) {
1244- out += ", Children list needs update, current count: " + std::to_string(fullChildrenList_->size());1244+ auto currentList = std::atomic_load_explicit(&fullChildrenList_, std::memory_order_acquire);
1245- if (!fullChildrenList_->empty()) {1245+ if (!currentList) {
1246+ return;
1247+ }
1248+ out += ", Children list needs update, current count: " + std::to_string(currentList->size());
1249+ if (!currentList->empty()) {
1246 int cnt = 0;1250 int cnt = 0;
1247 out += "(";1251 out += "(";
1248- for (auto child = fullChildrenList_->begin(); child != fullChildrenList_->end(); child++) {1252+ for (auto child = currentList->begin(); child != currentList->end(); child++) {
chuchengcheng
chuchengchengchuchengcheng7月23日

🤖 AI 代码检视意见 | 🛡️ Reliability | ℹ️ Medium | 行号区间: L1246-L1251

问题描述: PR 通过 std::atomic_load_explicit 获取 fullChildrenList_ 的共享副本 currentList,随后直接调用 currentList->size() 和 currentList->empty()。但同文件 RemoveChildFromFulllist (line 5090-5093) 对 atomic_load 结果显式做了 if (!currentList) { return; } 判空,说明 fullChildrenList_ 在某些生命周期阶段可能为 nullptr。ChildrenListDump 缺少同样的判空守卫,若在节点初始化前或清理后被调用,将触发空指针解引用崩溃,与 PR 提升稳定性的目标相悖。

💡 查看修复建议(点击展开)

修改建议:在 atomic_load 之后、解引用之前增加判空检查,与 RemoveChildFromFulllist 保持一致的防御性编程模式。

auto currentList = std::atomic_load_explicit(&fullChildrenList_, std::memory_order_acquire);
if (!currentList) {
return;
}
out += ", Children list needs update, current count: " + std::to_string(currentList->size());
if (!currentList->empty()) {
int cnt = 0;
out += "(";
for (auto child = currentList->begin(); child != currentList->end(); child++) {
if (cnt > childrenCntLimit) {
likedislike
1249 if (cnt > childrenCntLimit) {1253 if (cnt > childrenCntLimit) {
1250 break;1254 break;
sunriseLL
sunriseLLsunriseLL7月23日

已补充 ChildrenListDump 和 ResortChildren 中 atomic_load 后的判空守卫,与 RemoveChildFromFulllist 保持一致。EmptyChildrenList 在头文件中为 static const inline 非空 shared_ptr(rs_render_node.h:1335-1336),fullChildrenList_ 初始化即非空,判空为防御性保护。

likedislike
1251 }1255 }
@@ -4184,7 +4188,7 @@ void RSRenderNode::GenerateFullChildrenList()
4184{4188{
4185 // both children_ and disappearingChildren_ are empty, no need to generate fullChildrenList_4189 // both children_ and disappearingChildren_ are empty, no need to generate fullChildrenList_
4186 if (children_.empty() && disappearingChildren_.empty()) {4190 if (children_.empty() && disappearingChildren_.empty()) {
4187- auto prevFullChildrenList = fullChildrenList_;4191+ auto prevFullChildrenList = std::atomic_load_explicit(&fullChildrenList_, std::memory_order_acquire);
4188 isFullChildrenListValid_ = true;4192 isFullChildrenListValid_ = true;
chuchengcheng
chuchengchengchuchengcheng7月23日

🤖 AI 代码检视意见 | 🛡️ Reliability | ✅ Low | 行号区间: L4232-L4233

问题描述: GenerateFullChildrenList 在 children 都为空时通过 atomic_store_explicit 将 fullChildrenList_ 置为 EmptyChildrenList。需确认 EmptyChildrenList 是全局静态非空 shared_ptr,否则后续 atomic_load 获取的副本仍为空,叠加 ChildrenListDump/ResortChildren 缺少判空,会形成空指针解引用链。

💡 查看修复建议(点击展开)

修改建议:确认 EmptyChildrenList 为非空静态 shared_ptr,并在赋值后加断言;或在下游消费点统一加判空守卫。

std::atomic_store_explicit(&fullChildrenList_, EmptyChildrenList, std::memory_order_release);
return;
likedislike
4189 isChildrenSorted_ = true;4193 isChildrenSorted_ = true;
4190 std::atomic_store_explicit(&fullChildrenList_, EmptyChildrenList, std::memory_order_release);4194 std::atomic_store_explicit(&fullChildrenList_, EmptyChildrenList, std::memory_order_release);
@@ -4240,7 +4244,7 @@ void RSRenderNode::GenerateFullChildrenList()
4240 });4244 });
4241 4245 
4242 // Keep a reference to fullChildrenList_ to prevent its deletion when swapping it4246 // Keep a reference to fullChildrenList_ to prevent its deletion when swapping it
4243- auto prevFullChildrenList = fullChildrenList_;4247+ auto prevFullChildrenList = std::atomic_load_explicit(&fullChildrenList_, std::memory_order_acquire);
4244 4248 
4245 // Update the flag to indicate that children are now valid and sorted4249 // Update the flag to indicate that children are now valid and sorted
4246 isFullChildrenListValid_ = true;4250 isFullChildrenListValid_ = true;
@@ -4254,7 +4258,11 @@ void RSRenderNode::GenerateFullChildrenList()
4254void RSRenderNode::ResortChildren()4258void RSRenderNode::ResortChildren()
4255{4259{
4256 // Make a copy of the fullChildrenList for sorting4260 // Make a copy of the fullChildrenList for sorting
4257- auto fullChildrenList = std::make_shared<std::vector<std::shared_ptr<RSRenderNode>>>(*fullChildrenList_);4261+ auto currentList = std::atomic_load_explicit(&fullChildrenList_, std::memory_order_acquire);
4262+ if (!currentList) {
4263+ return;
4264+ }
4265+ auto fullChildrenList = std::make_shared<std::vector<std::shared_ptr<RSRenderNode>>>(*currentList);
chuchengcheng
chuchengchengchuchengcheng7月23日

🤖 AI 代码检视意见 | 🛡️ Reliability | ℹ️ Medium | 行号区间: L4302-L4303

问题描述: ResortChildren 通过 atomic_load_explicit 获取 currentList 后,立即用 *currentList 解引用构造新的 vector。若 fullChildrenList_ 为 nullptr,currentList 将为空 shared_ptr,解引用 *currentList 会触发空指针崩溃。同文件 RemoveChildFromFulllist 已有判空先例,此处应保持一致。

💡 查看修复建议(点击展开)

修改建议:在 atomic_load 后增加判空检查,返回或提前退出以避免空指针解引用。

auto currentList = std::atomic_load_explicit(&fullChildrenList_, std::memory_order_acquire);
if (!currentList) {
return;
}
auto fullChildrenList = std::make_shared<std::vector<std::shared_ptr<RSRenderNode>>>(*currentList);
likedislike
4258 4266 
4259 // temporary fix for wrong z-order4267 // temporary fix for wrong z-order
4260 for (auto& child : *fullChildrenList) {4268 for (auto& child : *fullChildrenList) {
@@ -4268,7 +4276,7 @@ void RSRenderNode::ResortChildren()
4268 });4276 });
4269 4277 
4270 // Keep a reference to fullChildrenList_ to prevent its deletion when swapping it4278 // Keep a reference to fullChildrenList_ to prevent its deletion when swapping it
4271- auto prevFullChildrenList = fullChildrenList_;4279+ auto prevFullChildrenList = std::atomic_load_explicit(&fullChildrenList_, std::memory_order_acquire);
4272 4280 
4273 // Update the flag to indicate that children are now sorted4281 // Update the flag to indicate that children are now sorted
4274 isChildrenSorted_ = true;4282 isChildrenSorted_ = true;
@@ -5041,10 +5049,11 @@ void RSRenderNode::SetChildrenHasSharedTransition(bool hasSharedTransition)
5041void RSRenderNode::RemoveChildFromFulllist(NodeId id)5049void RSRenderNode::RemoveChildFromFulllist(NodeId id)
5042{5050{
5043 // Make a copy of the fullChildrenList5051 // Make a copy of the fullChildrenList
5044- if (!fullChildrenList_) {5052+ auto currentList = std::atomic_load_explicit(&fullChildrenList_, std::memory_order_acquire);
5053+ if (!currentList) {
5045 return;5054 return;
5046 }5055 }
5047- auto fullChildrenList = std::make_shared<std::vector<std::shared_ptr<RSRenderNode>>>(*fullChildrenList_);5056+ auto fullChildrenList = std::make_shared<std::vector<std::shared_ptr<RSRenderNode>>>(*currentList);
5048 5057 
5049 fullChildrenList->erase(std::remove_if(fullChildrenList->begin(),5058 fullChildrenList->erase(std::remove_if(fullChildrenList->begin(),
5050 fullChildrenList->end(), [id](const auto& node) { return id == node->GetId(); }), fullChildrenList->end());5059 fullChildrenList->end(), [id](const auto& node) { return id == node->GetId(); }), fullChildrenList->end());
@@ -238,11 +238,6 @@ bool RSSystemProperties::GetCacheEnabledForRotation()
238 return {};238 return {};
239}239}
240 240 
241-ParallelRenderingType RSSystemProperties::GetPrepareParallelRenderingEnabled()
242-{
243- return {};
244-}
245- 
246ParallelRenderingType RSSystemProperties::GetParallelRenderingEnabled()241ParallelRenderingType RSSystemProperties::GetParallelRenderingEnabled()
247{242{
248 return {};243 return {};
@@ -374,7 +374,8 @@ void RSAshmemHelper::InjectFileDescriptor(std::shared_ptr<MessageParcel>& dataPa
374 uintptr_t data = dataParcel->GetData();374 uintptr_t data = dataParcel->GetData();
375 for (size_t i = 0; i < objectNum; i++) {375 for (size_t i = 0; i < objectNum; i++) {
376 binder_size_t offset = object[i];376 binder_size_t offset = object[i];
377- if (offset + sizeof(flat_binder_object) > dataParcel->GetDataSize()) {377+ size_t dataSize = dataParcel->GetDataSize();
378+ if (offset > dataSize || sizeof(flat_binder_object) > dataSize - offset) {
378 ROSEN_LOGW("RSAshmemHelper::InjectFileDescriptor offset invalid");379 ROSEN_LOGW("RSAshmemHelper::InjectFileDescriptor offset invalid");
379 continue;380 continue;
380 }381 }
@@ -193,14 +193,18 @@ bool RSClientToServiceConnectHub::Connect()
193 193 
194void RSClientToServiceConnectHub::ConnectDied()194void RSClientToServiceConnectHub::ConnectDied()
195{195{
196- std::lock_guard<std::mutex> lock(mutex_);196+ sptr<RSIClientToServiceConnection> conn;
197- renderService_ = nullptr;197+ {
198- if (conn_) {198+ std::lock_guard<std::mutex> lock(mutex_);
199- conn_->RunOnRemoteDiedCallback();199+ renderService_ = nullptr;
200+ conn = conn_;
201+ conn_ = nullptr;
202+ deathRecipient_ = nullptr;
203+ token_ = nullptr;
204+ }
205+ if (conn) {
206+ conn->RunOnRemoteDiedCallback();
L
LLyBbq7月23日

该接口,是否Set时候也加入锁,后续也用这个锁控制住

likedislike
sunriseLL
sunriseLL
7月23日 评论:
sunriseLL
sunriseLL
7月23日 评论:
200 }207 }
201- conn_ = nullptr;
202- deathRecipient_ = nullptr;
203- token_ = nullptr;
204}208}
205 209 
206void RSClientToServiceConnectHub::RenderServiceDeathRecipient::OnRemoteDied(const wptr<IRemoteObject>& remote)210void RSClientToServiceConnectHub::RenderServiceDeathRecipient::OnRemoteDied(const wptr<IRemoteObject>& remote)
@@ -29,10 +29,6 @@ class RSClientToServiceConnectHub : public RefBase {
29public:29public:
30 static sptr<RSIClientToServiceConnection> GetClientToServiceConnection();30 static sptr<RSIClientToServiceConnection> GetClientToServiceConnection();
31 static sptr<RSClientToServiceConnectHub> GetInstance();31 static sptr<RSClientToServiceConnectHub> GetInstance();
32- RSIConnectionToken* GetToken()
33- {
34- return token_.GetRefPtr();
35- }
36 void ConnectDied();32 void ConnectDied();
chuchengcheng
chuchengchengchuchengcheng7月23日

🤖 AI 代码检视意见 | 🏗️ Architecture | ℹ️ Medium | 行号区间: L32

问题描述: PR 描述仅声明移除 GetPrepareParallelRenderingEnabled,但实际同时移除了 RSClientToServiceConnectHub::GetToken() 和 RSRenderServiceConnectHub::GetToken() 两个公共接口。这些接口返回 RSIConnectionToken*,可能被同层渲染或外部模块调用。未在 PR 中说明即移除公共 API 属于接口契约破坏,需确认 OpenHarmony 全仓无调用方,否则将导致编译失败或运行时行为变更。

💡 查看修复建议(点击展开)

修改建议:在 PR 描述中补充移除 GetToken() 的理由和影响面,并在 CodeCheck 中确认全仓无残留调用;若仍有调用方,应提供替代接口或分阶段废弃。

void ConnectDied();

private:
likedislike
37 33 
38private:34private:
sunriseLL
sunriseLLsunriseLL7月23日

GetToken 已从两个 Hub 类中删除,无生产调用方,单测中的引用已清理。

likedislike
@@ -126,8 +126,12 @@ bool RSRenderPipelineClient::CreateDisplayNode(const RSDisplayNodeConfig& displa
126 ROSEN_LOGE("RSRenderPipelineClient::CreateNode clientToRenderConnection_ nullptr");126 ROSEN_LOGE("RSRenderPipelineClient::CreateNode clientToRenderConnection_ nullptr");
127 return false;127 return false;
128 }128 }
129- bool success;129+ bool success = false;
130- clientToRenderConnection->CreateDisplayNode(displayNodeConfig, nodeId, success);130+ ErrCode err = clientToRenderConnection->CreateDisplayNode(displayNodeConfig, nodeId, success);
131+ if (err != ERR_OK) {
132+ ROSEN_LOGE("RSRenderPipelineClient::CreateDisplayNode failed, err:%{public}d", err);
133+ return false;
134+ }
131 return success;135 return success;
132}136}
133 137 
@@ -138,8 +142,12 @@ bool RSRenderPipelineClient::CreateNode(const RSSurfaceRenderNodeConfig& config)
138 ROSEN_LOGE("RSRenderPipelineClient::CreateNode clientToRenderConnection_ nullptr");142 ROSEN_LOGE("RSRenderPipelineClient::CreateNode clientToRenderConnection_ nullptr");
139 return false;143 return false;
140 }144 }
141- bool success;145+ bool success = false;
142- clientToRenderConnection->CreateNode(config, success);146+ ErrCode err = clientToRenderConnection->CreateNode(config, success);
147+ if (err != ERR_OK) {
148+ ROSEN_LOGE("RSRenderPipelineClient::CreateNode failed, err:%{public}d", err);
149+ return false;
150+ }
143 return success;151 return success;
144}152}
145 153 
@@ -295,8 +303,11 @@ uint32_t RSRenderPipelineClient::SetHidePrivacyContent(NodeId id, bool needHideP
295{303{
296 auto clientToRenderConnection = RSRenderServiceConnectHub::GetClientToRenderConnection(tokenMaskId_);304 auto clientToRenderConnection = RSRenderServiceConnectHub::GetClientToRenderConnection(tokenMaskId_);
297 if (clientToRenderConnection != nullptr) {305 if (clientToRenderConnection != nullptr) {
298- uint32_t resCode;306+ uint32_t resCode = static_cast<uint32_t>(RSInterfaceErrorCode::UNKNOWN_ERROR);
299- clientToRenderConnection->SetHidePrivacyContent(id, needHidePrivacyContent, resCode);307+ ErrCode err = clientToRenderConnection->SetHidePrivacyContent(id, needHidePrivacyContent, resCode);
308+ if (err != ERR_OK) {
309+ ROSEN_LOGE("RSRenderPipelineClient::SetHidePrivacyContent failed, err:%{public}d", err);
310+ }
300 return resCode;311 return resCode;
301 }312 }
302 ROSEN_LOGE("RSRenderPipelineClient::SetHidePrivacyContent clientToRenderConnection_ is nullptr!");313 ROSEN_LOGE("RSRenderPipelineClient::SetHidePrivacyContent clientToRenderConnection_ is nullptr!");
@@ -90,10 +90,6 @@ public:
90 sptr<IRemoteObject> connectToRenderRemote,90 sptr<IRemoteObject> connectToRenderRemote,
91 sptr<RSIConnectToRenderProcess> renderPrecess,91 sptr<RSIConnectToRenderProcess> renderPrecess,
92 sptr<RSIClientToRenderConnection> clientToRenderConnection);92 sptr<RSIClientToRenderConnection> clientToRenderConnection);
93- RSIConnectionToken* GetToken()
94- {
95- return token_.GetRefPtr();
96- }
97 void ConnectRenderProcessDied(uint64_t tokenMaskId);93 void ConnectRenderProcessDied(uint64_t tokenMaskId);
98 // Temporary interface for same-layer rendering and non-multi-instance-adapted interfaces94 // Temporary interface for same-layer rendering and non-multi-instance-adapted interfaces
99 uint64_t GetDefaultTokenMaskIdInner();95 uint64_t GetDefaultTokenMaskIdInner();
@@ -608,13 +608,6 @@ bool RSSystemProperties::GetCacheEnabledForRotation()
608 return cacheEnabledForRotation_.load();608 return cacheEnabledForRotation_.load();
609}609}
610 610 
611-ParallelRenderingType RSSystemProperties::GetPrepareParallelRenderingEnabled()
612-{
613- static ParallelRenderingType systemPropertiePrepareType = static_cast<ParallelRenderingType>(
614- std::atoi((system::GetParameter("persist.rosen.prepareparallelrender.enabled", "1")).c_str()));
615- return systemPropertiePrepareType;
616-}
617- 
618ParallelRenderingType RSSystemProperties::GetParallelRenderingEnabled()611ParallelRenderingType RSSystemProperties::GetParallelRenderingEnabled()
619{612{
620 static ParallelRenderingType systemPropertieType = static_cast<ParallelRenderingType>(613 static ParallelRenderingType systemPropertieType = static_cast<ParallelRenderingType>(
@@ -4605,13 +4605,19 @@ ErrCode RSClientToServiceConnectionProxy::SetCacheEnabledForRotation(bool isEnab
4605#endif4605#endif
4606void RSClientToServiceConnectionProxy::SetOnRemoteDiedCallback(const OnRemoteDiedCallback& callback)4606void RSClientToServiceConnectionProxy::SetOnRemoteDiedCallback(const OnRemoteDiedCallback& callback)
4607{4607{
4608+ std::lock_guard<std::mutex> lock(onRemoteDiedCallbackMutex_);
4608 OnRemoteDiedCallback_ = callback;4609 OnRemoteDiedCallback_ = callback;
4609}4610}
4610 4611 
4611void RSClientToServiceConnectionProxy::RunOnRemoteDiedCallback()4612void RSClientToServiceConnectionProxy::RunOnRemoteDiedCallback()
4612{4613{
4613- if (OnRemoteDiedCallback_) {4614+ OnRemoteDiedCallback callback;
4614- OnRemoteDiedCallback_();4615+ {
4616+ std::lock_guard<std::mutex> lock(onRemoteDiedCallbackMutex_);
4617+ callback = OnRemoteDiedCallback_;
4618+ }
4619+ if (callback) {
4620+ callback();
4615 }4621 }
4616}4622}
4617 4623 
@@ -18,6 +18,7 @@
18 18 
19#include <iremote_proxy.h>19#include <iremote_proxy.h>
20#include <memory>20#include <memory>
21+#include <mutex>
21#include <platform/ohos/transaction/zidl/rs_iclient_to_service_connection.h>22#include <platform/ohos/transaction/zidl/rs_iclient_to_service_connection.h>
22#include <platform/ohos/transaction/rs_iclient_to_service_connection_ipc_interface_code.h>23#include <platform/ohos/transaction/rs_iclient_to_service_connection_ipc_interface_code.h>
23#ifndef ENABLE_RS_PROXY24#ifndef ENABLE_RS_PROXY
@@ -401,6 +402,7 @@ private:
401 std::atomic<uint32_t> transactionDataIndex_ = 0;402 std::atomic<uint32_t> transactionDataIndex_ = 0;
402#endif403#endif
403 OnRemoteDiedCallback OnRemoteDiedCallback_;404 OnRemoteDiedCallback OnRemoteDiedCallback_;
405+ std::mutex onRemoteDiedCallbackMutex_;
404};406};
405} // namespace Rosen407} // namespace Rosen
406} // namespace OHOS408} // namespace OHOS
@@ -242,11 +242,6 @@ bool RSSystemProperties::GetCacheEnabledForRotation()
242 return {};242 return {};
243}243}
244 244 
245-ParallelRenderingType RSSystemProperties::GetPrepareParallelRenderingEnabled()
246-{
247- return {};
248-}
249- 
250ParallelRenderingType RSSystemProperties::GetParallelRenderingEnabled()245ParallelRenderingType RSSystemProperties::GetParallelRenderingEnabled()
251{246{
252 return {};247 return {};
@@ -73,19 +73,6 @@ HWTEST_F(RSClientToServiceConnectHubTest, GetClientToServiceConnection001, TestS
73 EXPECT_NE(conn, nullptr);73 EXPECT_NE(conn, nullptr);
74}74}
75 75 
76-/**
77- * @tc.name: GetToken001
78- * @tc.desc: Verify GetToken returns nullptr when not connected
79- * @tc.type: FUNC
80- */
81-HWTEST_F(RSClientToServiceConnectHubTest, GetToken001, TestSize.Level1)
82-{
83- auto instance = RSClientToServiceConnectHub::GetInstance();
84- ASSERT_NE(instance, nullptr);
85- auto token = instance->GetToken();
86- EXPECT_NE(token, nullptr);
87-}
88- 
89/**76/**
90 * @tc.name: ConnectDied00177 * @tc.name: ConnectDied001
91 * @tc.desc: Verify ConnectDied does not crash when not connected78 * @tc.desc: Verify ConnectDied does not crash when not connected
@@ -96,7 +83,6 @@ HWTEST_F(RSClientToServiceConnectHubTest, ConnectDied001, TestSize.Level1)
96 auto instance = RSClientToServiceConnectHub::GetInstance();83 auto instance = RSClientToServiceConnectHub::GetInstance();
97 ASSERT_NE(instance, nullptr);84 ASSERT_NE(instance, nullptr);
98 instance->ConnectDied();85 instance->ConnectDied();
99- EXPECT_EQ(instance->GetToken(), nullptr);
100}86}
101 87 
102/**88/**
@@ -123,8 +109,6 @@ HWTEST_F(RSClientToServiceConnectHubTest, ConnectDied002, TestSize.Level1)
123 auto instance = RSClientToServiceConnectHub::GetInstance();109 auto instance = RSClientToServiceConnectHub::GetInstance();
124 ASSERT_NE(instance, nullptr);110 ASSERT_NE(instance, nullptr);
125 instance->ConnectDied();111 instance->ConnectDied();
126- auto tokenAfter = instance->GetToken();
127- EXPECT_EQ(tokenAfter, nullptr);
128}112}
129 113 
130/**114/**
@@ -330,6 +330,25 @@ HWTEST_F(RSSurfaceNodeCommandTest, TestRSSurfaceNodeCommand013, TestSize.Level1)
330 ASSERT_EQ(id2, static_cast<NodeId>(10));330 ASSERT_EQ(id2, static_cast<NodeId>(10));
331}331}
332 332 
333+/**
334+ * @tc.name: SetSurfaceNodeTypeInvalidTest
335+ * @tc.desc: Verify SetSurfaceNodeType rejects out-of-range enum value
336+ * @tc.type: FUNC
337+ */
338+HWTEST_F(RSSurfaceNodeCommandTest, SetSurfaceNodeTypeInvalidTest, TestSize.Level1)
339+{
340+ RSContext context;
341+ NodeId id = 20;
342+ SurfaceNodeCommandHelper::Create(context, id);
343+ uint8_t invalidType = static_cast<uint8_t>(RSSurfaceNodeType::NODE_MAX);
344+ SurfaceNodeCommandHelper::SetSurfaceNodeType(context, id, invalidType);
345+ uint8_t overRangeType = static_cast<uint8_t>(RSSurfaceNodeType::NODE_MAX) + 10;
346+ SurfaceNodeCommandHelper::SetSurfaceNodeType(context, id, overRangeType);
347+ auto node = context.GetNodeMap().GetRenderNode<RSSurfaceRenderNode>(id);
348+ ASSERT_NE(node, nullptr);
349+ EXPECT_EQ(node->GetSurfaceNodeType(), RSSurfaceNodeType::DEFAULT);
350+}
351+ 
333/**352/**
334 * @tc.name: TestRSSurfaceNodeCommand015353 * @tc.name: TestRSSurfaceNodeCommand015
335 * @tc.desc: SetContextAlpha test.354 * @tc.desc: SetContextAlpha test.
@@ -2759,5 +2759,35 @@ HWTEST_F(RSMarshallingHelperTest, IRemoteObjectMarshallingRoundTripTest, TestSiz
2759 ASSERT_TRUE(dstVal != nullptr);2759 ASSERT_TRUE(dstVal != nullptr);
2760}2760}
2761#endif2761#endif
2762+ 
2763+/**
2764+ * @tc.name: UnmarshallingMapExceedMaxSizeTest
2765+ * @tc.desc: Verify std::map Unmarshalling rejects oversized size
2766+ * @tc.type: FUNC
2767+ */
2768+HWTEST_F(RSMarshallingHelperTest, UnmarshallingMapExceedMaxSizeTest, TestSize.Level1)
2769+{
2770+ Parcel parcel;
2771+ uint32_t oversized = static_cast<uint32_t>(RSMarshallingHelper::UNMARSHALLING_MAX_VECTOR_SIZE) + 1;
2772+ ASSERT_TRUE(RSMarshallingHelper::Marshalling(parcel, oversized));
2773+ std::map<int, int> val;
2774+ EXPECT_FALSE(RSMarshallingHelper::Unmarshalling(parcel, val));
2775+ EXPECT_TRUE(val.empty());
2776+}
2777+ 
2778+/**
2779+ * @tc.name: MarshallingMapExceedMaxSizeTest
2780+ * @tc.desc: Verify std::map Marshalling rejects oversized map
2781+ * @tc.type: FUNC
2782+ */
2783+HWTEST_F(RSMarshallingHelperTest, MarshallingMapExceedMaxSizeTest, TestSize.Level1)
2784+{
2785+ Parcel parcel;
2786+ std::map<int, int> val;
2787+ for (int i = 0; i <= static_cast<int>(RSMarshallingHelper::UNMARSHALLING_MAX_VECTOR_SIZE); ++i) {
2788+ val[i] = i;
2789+ }
2790+ EXPECT_FALSE(RSMarshallingHelper::Marshalling(parcel, val));
2791+}
2762} // namespace Rosen2792} // namespace Rosen
2763} // namespace OHOS2793} // namespace OHOS
@@ -495,17 +495,6 @@ HWTEST_F(RSSystemPropertiesTest, GetCacheEnabledForRotation, TestSize.Level1)
495 ASSERT_TRUE(RSSystemProperties::GetCacheEnabledForRotation());495 ASSERT_TRUE(RSSystemProperties::GetCacheEnabledForRotation());
496}496}
497 497 
498-/**
499- * @tc.name: GetPrepareParallelRenderingEnabled
500- * @tc.desc: GetPrepareParallelRenderingEnabled Test
501- * @tc.type:FUNC
502- * @tc.require: issueI9JZWC
503- */
504-HWTEST_F(RSSystemPropertiesTest, GetPrepareParallelRenderingEnabled, TestSize.Level1)
505-{
506- ASSERT_EQ(RSSystemProperties::GetPrepareParallelRenderingEnabled(), ParallelRenderingType::DISABLE);
507-}
508- 
509/**498/**
510 * @tc.name: GetParallelRenderingEnabled499 * @tc.name: GetParallelRenderingEnabled
511 * @tc.desc: GetParallelRenderingEnabled Test500 * @tc.desc: GetParallelRenderingEnabled Test