在一次实际开发中遇到程序崩溃问题,代码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()
{
加锁
写共享数据
局部变量读共享数据
解锁
耗时操作
读局部变量
}
巧妙的利用局部变量保存了读共享数据,使写、读共享数据在一起,中间没有了耗时操作。
问题终于解决!