Star5
96
代码
代码
Issues40
Pull Requests26
项目讨论
Wiki
分析
Star5
96
已合并
分布式回调异步初始化并增加TimeServiceClient空指针校验 #5017
stepend98创建于 12 天前
分布式回调异步初始化并增加TimeServiceClient空指针校验 #5017
已合并
stepend98创建于 12 天前
从stepend98/notification_distributed_notification_servicefix/ans-startup-robustness合入到openharmony/notification_distributed_notification_servicemaster
共 2 个文件变更+18-6
Mservices/ans/src/advanced_notification_service.cpp+6-4
@@ -373,10 +373,12 @@ AdvancedNotificationService::AdvancedNotificationService()
373 systemEventObserver_ = std::make_shared<SystemEventObserver>(iSystemEvent);373 systemEventObserver_ = std::make_shared<SystemEventObserver>(iSystemEvent);
374 DelayedSingleton<NotificationConfigParse>::GetInstance()->GetReportTrustListConfig();374 DelayedSingleton<NotificationConfigParse>::GetInstance()->GetReportTrustListConfig();
375#ifdef ANS_FEATURE_ORIGINAL_DISTRIBUTED375#ifdef ANS_FEATURE_ORIGINAL_DISTRIBUTED
376- distributedKvStoreDeathRecipient_ = std::make_shared<DistributedKvStoreDeathRecipient>(376+ notificationSvrQueue_.Submit([this]() {
A
Aafwk_helper12 天前

🤖 AI 代码检视意见（回复本评论可解决检视意见，点击被检视代码行左侧的小头像可收起检视意见）

🟠 构造函数中异步捕获 this 存在生命周期与竞态风险

位置: L379-L382 | 严重程度: High

❓ 问题描述

在构造函数中向队列提交异步任务并捕获 this，存在两个风险：1) 如果对象构造失败或被提前销毁，异步任务执行时 this 将成为悬空指针；2) 异步执行 InitDistributeCallBack() 会导致对象在构造完成后处于未完全初始化状态，若其他线程在此期间访问该对象依赖的分布式回调资源，将引发竞态条件。

💡 修复建议

修改建议：建议将初始化逻辑移至独立的 Init() 方法中在 SA 启动后调用，避免在构造函数中异步执行导致竞态。若确需在构造函数中异步执行，应确保所有依赖该回调的接口均通过 notificationSvrQueue_ 串行化调度。

379: // 建议移至 Init() 方法中同步或异步调用，确保生命周期安全 380: // notificationSvrQueue_.Submit(this { 381: // ANS_LOGI("Init distribute callback asynchronously."); 382: // InitDistributeCallBack(); 383: // });

likedislike
stepend98
stepend98
6 天前 评论：
377- std::bind(&AdvancedNotificationService::OnDistributedKvStoreDeathRecipient, this));377+ distributedKvStoreDeathRecipient_ = std::make_shared<DistributedKvStoreDeathRecipient>(
378- dataManager_.RegisterKvStoreServiceDeathRecipient(distributedKvStoreDeathRecipient_);378+ std::bind(&AdvancedNotificationService::OnDistributedKvStoreDeathRecipient, this));
379- InitDistributeCallBack();379+ dataManager_.RegisterKvStoreServiceDeathRecipient(distributedKvStoreDeathRecipient_);
380+ InitDistributeCallBack();
381+ });
380#endif382#endif
381#ifdef ANS_FEATURE_NOTIFICATION_STATISTICS383#ifdef ANS_FEATURE_NOTIFICATION_STATISTICS
382 NotificationAnalyticsUtil::CreateCleanExperDataTimerExecute();384 NotificationAnalyticsUtil::CreateCleanExperDataTimerExecute();
Mservices/ans/src/system_event_observer.cpp+12-2
@@ -98,7 +98,12 @@ SystemEventObserver::~SystemEventObserver()
98void SystemEventObserver::InitSaStartTime()98void SystemEventObserver::InitSaStartTime()
99{99{
100 g_saStartTime = NotificationAnalyticsUtil::GetCurrentTime();100 g_saStartTime = NotificationAnalyticsUtil::GetCurrentTime();
101- g_saStartBootTime = MiscServices::TimeServiceClient::GetInstance()->GetBootTimeMs();101+ sptr<MiscServices::TimeServiceClient> timer = MiscServices::TimeServiceClient::GetInstance();
A
Aafwk_helper12 天前

🤖 AI 代码检视意见（回复本评论可解决检视意见，点击被检视代码行左侧的小头像可收起检视意见）

🔴 隐式构造 sptr 可能导致单例被错误销毁

位置: L101-L106 | 严重程度: Critical

❓ 问题描述

TimeServiceClient::GetInstance() 返回的是原始指针（由原代码的 -> 操作符可知）。将其赋值给 sptr<MiscServices::TimeServiceClient> 触发了从原始指针隐式构造智能指针，这违反了平台规范。更严重的是，当 timer 离开作用域时，sptr 会尝试减少引用计数甚至销毁该单例对象，如果该对象不是通过 RefBase 管理的或为静态局部变量，将导致重复释放或程序崩溃。

💡 修复建议

修改建议：避免使用 sptr 接管 GetInstance() 返回的原始指针，直接使用原始指针并在使用前进行非空校验。

101: MiscServices::TimeServiceClient* timer = MiscServices::TimeServiceClient::GetInstance(); 102: if (timer == nullptr) { 103: ANS_LOGE("TimeServiceClient is null, failed to init SA start boot time."); 104: return; 105: } 106: g_saStartBootTime = timer->GetBootTimeMs();

likedislike
stepend98
stepend98
6 天前 评论：
102+ if (timer == nullptr) {
103+ ANS_LOGE("TimeServiceClient is null, failed to init SA start boot time.");
104+ return;
105+ }
106+ g_saStartBootTime = timer->GetBootTimeMs();
102}107}
103 108 
104sptr<NotificationBundleOption> SystemEventObserver::GetBundleOption(AAFwk::Want want)109sptr<NotificationBundleOption> SystemEventObserver::GetBundleOption(AAFwk::Want want)
@@ -141,7 +146,12 @@ void SystemEventObserver::OnReceiveEvent(const EventFwk::CommonEventData &data)
141 if (action == EventFwk::CommonEventSupport::COMMON_EVENT_TIME_CHANGED ||146 if (action == EventFwk::CommonEventSupport::COMMON_EVENT_TIME_CHANGED ||
142 action == EventFwk::CommonEventSupport::COMMON_EVENT_TIMEZONE_CHANGED) {147 action == EventFwk::CommonEventSupport::COMMON_EVENT_TIMEZONE_CHANGED) {
143 int64_t current = NotificationAnalyticsUtil::GetCurrentTime();148 int64_t current = NotificationAnalyticsUtil::GetCurrentTime();
144- int64_t bootTimeMs = MiscServices::TimeServiceClient::GetInstance()->GetBootTimeMs();149+ sptr<MiscServices::TimeServiceClient> timer = MiscServices::TimeServiceClient::GetInstance();
A
Aafwk_helper12 天前

🤖 AI 代码检视意见（回复本评论可解决检视意见，点击被检视代码行左侧的小头像可收起检视意见）

🔴 隐式构造 sptr 可能导致单例被错误销毁

位置: L149-L154 | 严重程度: Critical

❓ 问题描述

与 InitSaStartTime 类似，OnReceiveEvent 中同样将 GetInstance() 返回的原始指针赋值给 sptr，违反了智能指针隐式构造的禁令，并可能在 sptr 析构时错误销毁单例。

💡 修复建议

修改建议：直接使用原始指针进行非空校验，避免生命周期被 sptr 错误接管。

149: MiscServices::TimeServiceClient* timer = MiscServices::TimeServiceClient::GetInstance(); 150: if (timer == nullptr) { 151: ANS_LOGE("TimeServiceClient is null, failed to get boot time."); 152: return; 153: } 154: int64_t bootTimeMs = timer->GetBootTimeMs();

likedislike
stepend98
stepend98
6 天前 评论：
150+ if (timer == nullptr) {
151+ ANS_LOGE("TimeServiceClient is null, failed to get boot time.");
152+ return;
153+ }
154+ int64_t bootTimeMs = timer->GetBootTimeMs();
145 int64_t realTime = g_saStartTime + (bootTimeMs - g_saStartBootTime);155 int64_t realTime = g_saStartTime + (bootTimeMs - g_saStartBootTime);
146 int64_t diffTime = current - realTime;156 int64_t diffTime = current - realTime;
147 g_saStartTime = current;157 g_saStartTime = current;