12 Star 25 Fork 46

OpenHarmony / distributeddatamgr_datamgr

 / 详情

代码中的暴力操作,建议分析并优化

Declined
Bug
Opened this issue  
2021-08-07 11:08

【任务描述】

https://gitee.com/openharmony/distributeddatamgr_datamgr/blob/master/services/distributeddataservice/adapter/account/src/account_delegate_impl.cpp

void AccountDelegateImpl::SubscribeAccountEvent()
{
    ZLOGI("Subscribe account event listener start.");
    MatchingSkills matchingSkills;
    matchingSkills.AddEvent(CommonEventSupport::COMMON_EVENT_HWID_LOGIN);
    matchingSkills.AddEvent(CommonEventSupport::COMMON_EVENT_HWID_LOGOUT);
    matchingSkills.AddEvent(CommonEventSupport::COMMON_EVENT_HWID_TOKEN_INVALID);
    matchingSkills.AddEvent(CommonEventSupport::COMMON_EVENT_USER_REMOVED);
    CommonEventSubscribeInfo info(matchingSkills);
    eventSubscriber_ = std::make_shared<EventSubscriber>(info);
    eventSubscriber_->SetEventCallback([&](AccountEventInfo &account) {
        account.harmonyAccountId = GetCurrentHarmonyAccountId();
        NotifyAccountChanged(account);
    });

    std::thread th = std::thread([&]() {
        int tryTimes = 0;
 **        constexpr int MAX_RETRY_TIME = 300;** 

        constexpr int RETRY_WAIT_TIME_S = 1;

        // we use this method to make sure register success
        while (tryTimes < MAX_RETRY_TIME) {
            auto result = CommonEventManager::SubscribeCommonEvent(eventSubscriber_);
            if (result) {
                break;
            }

            ZLOGE("EventManager: Fail to register subscriber, error:%d", result);
            sleep(RETRY_WAIT_TIME_S);
            tryTimes++;
        }
        if (tryTimes == MAX_RETRY_TIME) {
            ZLOGE("EventManager: Fail to register subscriber!");
        }
        ZLOGI("EventManager: Success to register subscriber.");
    });
    th.detach();
}

大概率场景下,如果有异常,重试300次和3次应该是没有啥差异的,请分析下300次是否真的必须,以及评估下对系统的负面影响。

并同步排查下其它地方是否有类似不合理的代码进行同步修改。

【解决方案】

注册理应一次成功,如果有失败基本都是代码问题,暴露问题分析解决,即便重试也不应该重试这么多次,需纠正编程中的牛角尖和极端思维。对于类似暴力操作,建议分析优化。

【任务来源】

检视

Comments (3)

Zhangfeng created安全问题
Zhangfeng set top level to High
Zhangfeng set priority to Serious
Zhangfeng changed description
Zhangfeng changed issue type from 安全问题 to 缺陷
Zhangfeng changed top level from High to Not top
Sven Wang set assignee to Sven Wang
Expand operation logs

"有异常,重试300次": 主要是在订阅失败时,common event service可能还没有启动就绪,需要在5分钟内重试,保证common event service启动成功后,能最快速的注册成功,接入业务。

CommonEventManager::SubscribeCommonEvent(eventSubscriber_)这个接口是跨进行的IPC接口。
不存在失败一定是编码问题的一说。

Sven Wang changed priority from Serious to Unimportant
Sven Wang changed priority from Unimportant to Secondary

"有异常,重试300次": 主要是在订阅失败时,common event service可能还没有启动就绪,需要在5分钟内重试,保证common event service启动成功后,能最快速的注册成功,接入业务。

CommonEventManager::SubscribeCommonEvent(eventSubscriber_)这个接口是跨进行的IPC接口。
不存在失败一定是编码问题的一说。

@Sven Wang 其实可以考虑下双向机制,目标进程没起来导致注册失败的话,等进程起来后给发个消息或者通知,接收通知再处理。开源代码,当前的这种写法,毕竟。。。

1、CommonEventManager::SubscribeCommonEvent本身就是事件通知服务。如果再新增一个事件通知机制不合适。

2、就算再做一套事件通知机制,也是需要订阅发布模式。重试机制也属于当前这种逻辑,没必要写一套重复机制,也没收益。

紫丁花絮 changed issue state from 待办的 to 技术评审中
Sven Wang changed issue state from 技术评审中 to 已拒绝

Sign in to comment

Status
Assignees
Projects
Milestones
Pull Requests
Successfully merging a pull request will close this issue.
Branches
Planed to start   -   Planed to end
-
Top level
Priority
Duration (hours)
Confirm
参与者(2)
1
https://git.oschina.net/openharmony/distributeddatamgr_datamgr.git
git@git.oschina.net:openharmony/distributeddatamgr_datamgr.git
openharmony
distributeddatamgr_datamgr
distributeddatamgr_datamgr

Search