不易觉察的程序BUG

不易觉察的程序BUG

今天调试过程中发现了一个BUG，因为这种BUG具有一定的普遍性，所以记录下来。

代码如下：

// 添加状态
int CBuffManager::AddBuff(int iThingID)
{
    int iSlot = GetEmptySlot();
    if (iSlot < 0)
    {
        TRACESVR("Bug : buff reach max number %d.\n", iSlot);
        return-1;
    }
 
    CBuffObj* pBuffObj = CreateBuffObj();
    if (!pBuffObj)
    {
        TRACESVR("Bug : create buff fail, %d.", MAX_BUFF_OBJECT_NUMBER_IN_ZONE);
        return-2;
    }
    pBuffObj->Init(m_uiUin, iThingID);
    m_apiBuff[iSlot] = pBuffObj->GetObjectID();
 
    return 0;
}

在这段代码中下面语句的意思是先初始BUFF对象属性，然后将BUFF对象加入BUFF管理器。在之前的处理中，这段代码是OK的。

    pBuffObj->Init(m_uiUin, iThingID);          ---1
    m_apiBuff[iSlot] = pBuffObj->GetObjectID();---2

但是今天来了个新需求：在人物属性改变的时候必须将改变的属性通知客户端，而BUFF对象的添加会改变人物属性，即在语句1中改变了人物属性并通知给客户端，在语句2中将BUFF加入管理器。

出现的问题是这样的：通知给客户端的属性值还是人物之前的属性值，在服务器端用gdb打断点之后查看人物属性已经改过来了，为什么在通知的时候给的人物属性值却是之前的值呢？

通知客户端的代码如下：

// 通知客户端玩家新属性
int CMsgHelper::SendUACNotify(CRoleObj*pRoleObj, EUnitAttributeTypeeAttribute)
{
    ASSERT_AND_LOG_RTN_INT(pRoleObj);
   
    CMsgHelper::GenerateMsgHead(m_stMsg,MSGID_UAC_NOTIFY);
 
    // 准备消息结构体
…省略若干
    switch (eAttribute)
    {
    case EUAI_SPEED:
        {
            rstNotify.m_aiCurrentValue[0] = pRoleObj->GetSpeedID();
            break;
        }
    default:
        {
            rstNotify.m_aiCurrentValue[0] = pRoleObj->GetAttribute(eAttribute);
        }
    }
 
    return CMsgHelper::SendMsgToRoom(m_stMsg, pRoleObj);
}

这里BUFF产生的速度属性，我们就跟到了函数GetSpeedID():

// 当前实际速度（加BUFF）获取
int CRoleObj::GetSpeedID()
{
    int iSpeedID = m_stBuffManager.GetConstSpeed();
    if (0 != iSpeedID)
    {   
        return iSpeedID;
    }
 
    return GetAttribute(EUAI_SPEED);
}

跟到这里的时候，明眼就看出问题来了，在这里速度取值依赖BUFF管理器实时计算，而在第一段代码处理时，是先初始化BUFF对象，再加入BUFF管理器。所以在这里计算的时候对应的加速度BUFF对象还未加入BUFF管理器，计算出来人物属性值就还是原值。

最简单的修改办法是将这两行代码对调一下顺序，问题就解决了。

然而再回头看看，这样子做有什么风险呢？

即先把BUFF对象加入BUFF管理器，再进行初始化有什么不好的地方？

假设现在还有一个进程需要访问BUFF管理器，这里就可能遍历到一个没有初始化的BUFF对象，用未初始化对象进行处理可想而知会产生什么严重的后果了。

而目前该程序是单线程且该变量为局部变量可以保证没有问题。

如果是多线程或多进程访问，就必须保证是先初始化对象再加入对象管理器，或者在这两个操作加锁，但显然只有加入BUFF管理器这个操作需要加锁，初始化BUFF对象是不需要加锁的，如果两个操作都加锁就有可能影响程序效率。

另外一个问题是：在初始化BUFF对象中又再去访问BUFF管理器（需要通知客户端属性更改），这不是一个好的编程习惯。在这里由于是BUFF对象初始化完成之后再去调用通知客户端的代码，所以不会有问题。

更保险的一种做法是：在语句1和语句2之后再去依次判断哪些属性改变了，然后通知给客户端，这样做的弊端是需要同之前保存的旧值做比较才知道哪些属性改变了，另一种办法是在改变属性时设置一个标志位表明已经改变，在通知客户端时根据这些标志位获取改变值，还要在通知之后清除这些标志位。