记一次多线程问题引发的崩溃

在一次实际开发中遇到程序崩溃问题，代码demo如下：

#include <iostream>
#include <map>
#include <thread>
#include <mutex>
#include <atomic>
using namespace std;

//网络回调类
class NetworkCB
{
public:
    virtual void OnRecv(const char* pData) = 0;
};

//客户端类
class Client
{
public:
    void Connect(NetworkCB* p)
    {
        //模拟网络接收
        thread([ = ]()
        {
            while(true)
            {
                p->OnRecv("data");
                this_thread::sleep_for(chrono::milliseconds(1000));
            }

        }).detach();
    }
};

//直播类
class Live: public NetworkCB
{
public:
    void OnRecv(const char* pData)
    {
        cout << __FUNCTION__ << endl;
    }

    void Open()
    {
        m_Client.Connect(this);
    }

private:
    Client m_Client;
};

//视频管理类
class VideoManager
{
public:
    //打开视频
    int OpenVideo()
    {
        ++m_nVideoId;

        shared_ptr<Live> pLive = make_shared<Live>();

        pLive->Open();

        lock_guard<mutex> mapLock(m_mapMutex);
        m_mapVideo[m_nVideoId] = pLive;

        return m_nVideoId;
    }

private:
    //视频索引
    atomic<int>                m_nVideoId = 0;
    //map互斥量
    mutex                      m_mapMutex;
    //保存视频的map
    map<int, shared_ptr<Live>> m_mapVideo;
};

int main()
{
    VideoManager oVideoManager;

    //模拟100个并发
    for(int i = 0; i < 100; ++i)
    {
        thread([&]()
        {
            oVideoManager.OpenVideo();

        }).detach();
    }

    getchar();

    return 0;
}

调试运行程序崩溃：

这个问题我看了一上午找不到原因。

拉来同组的技术大佬老刘一起来看，

老刘右手捋了捋胡子说：“这个VideoManager::OpenVideo函数里面，m_nVideoId是原子类型，m_mapVideo加了锁，看着好像没问题，多半是你这个锁没加好，要不你在最前面加一个大锁试试”

于是我照做试了试，修改代码如下：

//视频管理类
class VideoManager
{
public:
    //打开视频
    int OpenVideo()
    {
        lock_guard<mutex> VideoIdLock(m_VideoIdMutex);

        ++m_nVideoId;

        shared_ptr<Live> pLive = make_shared<Live>();

        pLive->Open();

        lock_guard<mutex> mapLock(m_mapMutex);
        m_mapVideo[m_nVideoId] = pLive;

        return m_nVideoId;
    }

private:
    //VideoId互斥量
    mutex                      m_VideoIdMutex;
    //视频索引
    int                        m_nVideoId = 0;
    //map互斥量
    mutex                      m_mapMutex;
    //保存视频的map
    map<int, shared_ptr<Live>> m_mapVideo;
};

我在VideoManager::OpenVideo函数开头加了一个锁m_nVideoId的大锁，m_nVideoId改为非原子类型，果然问题解决了，不崩了，不愧是老刘啊。

突然，小组长把脸凑了过来问到：“你们两个在看啥呢？”

描述了一通后，小组长说：“你这肯定不行啊，m_nVideoId的锁范围过大了，实际开发中pLive->Open()的耗时可能需要几百毫秒，这个锁的范围这么大，性能肯定要受影响啊”

我和老刘面面相觑，一时说不出话来。

小组长一脸不屑的说：“哎，让开，我来给你们改改”。

一顿操作，修改代码如下：

//视频管理类
class VideoManager
{
public:
    //打开视频
    int OpenVideo()
    {
        m_VideoIdMutex.lock();
        int nTempVideoId = ++m_nVideoId;;
        m_VideoIdMutex.unlock();

        shared_ptr<Live> pLive = make_shared<Live>();

        pLive->Open();

        lock_guard<mutex> mapLock(m_mapMutex);

        m_mapVideo[nTempVideoId] = pLive;

        return nTempVideoId;
    }

private:
    //VideoId互斥量
    mutex                      m_VideoIdMutex;
    //视频索引
    int                        m_nVideoId = 0;
    //map互斥量
    mutex                      m_mapMutex;
    //保存视频的map
    map<int, shared_ptr<Live>> m_mapVideo;
};

突然老刘拍了一下桌子，大喊一声：“妙啊，妙啊”。

此时还在蒙蔽状态的我，看着代码心里想着：哪里妙了？

小组长看着我二人，开始了教学模式，说到：“首先分析崩溃原因，问题在VideoManager::OpenVideo这个函数，m_nVideoId是原子类型，m_mapVideo加了锁，但是++m_nVideoId到m_mapVideo[m_nVideoId] = pLive不是原子操作，同一个线程两次m_nVideoId可能不等，然后两个线程相同的m_nVideoId加入了map，但是map的key是唯一的，导致先加进map那个m_nVideoId的智能指针指向的对象释放，然而网络这边还在接收数据处理，就出现访问非法内存导致崩溃。其次，我们知道一个函数多线程运行的时候，只有共享的数据有线程安全问题，而其他的，比如局部变量就没有线程安全问题。VideoManager::OpenVideo函数两次访问m_nVideoId，一次是写，一次是读，中间又有耗时的pLive->Open()，于是想到用局部变量来保存m_nVideoId读这次操作，这样，写、读m_nVideoId在一起，而且在锁里面。避免了加锁范围过大带来的性能问题，同时也解决了程序崩溃问题“。

这个问题我们从头到尾用伪代码再分析一遍，看透问题的本质。

问题简化如下：

void fun()
{
    加锁
    写共享数据
    解锁

    耗时操作

    加锁
    读共享数据
    解锁
}

问题在于一个线程两次访问的共享数据不一致，写共享数据和读共享数据不是一个原子操作。

修改为如下：

void fun()
{
    加锁
    写共享数据

    耗时操作

    读共享数据
    解锁
}

写共享数据和读共享数据现在是一个原子操作了，但是由于中间有耗时操作，导致性能低下。

所以，假如写共享数据、读共享数据中间没有耗时操作就好了

修改为如下：

void fun()
{
    加锁
    写共享数据
    局部变量读共享数据
    解锁

    耗时操作
    读局部变量
}

巧妙的利用局部变量保存了读共享数据，使写、读共享数据在一起，中间没有了耗时操作。

问题终于解决！