大约在1个月前,游戏终于上线了,在这一个月以来,服务器竟然crash了5+次,还有几次严重Bug.
除了觉得测试力度不够之外,我也在想到底有那些环节是我能做而没做好的。
仔细思考下来,出Bug的原因大概有两种典型情况。
写代码时,逻辑思维不严谨,并且函数之间耦合性过强,导致随便一个很平常的改动,都会产生新的Bug, 甚至是crash. 比如我经常写出类似下面这种函数之间耦合性过强代码。
struct foo {
bool isfinish;
time_t rsttime;
//...
};
static viod try_reset(struct foo *f) {
if (f->rsttime < time(NULL)) {
//reset foo fields
}
}
static int bar(struct foo *f) {
if (f->isfinish)
return -1;
try_reset(f);
//....do more things
}
这上面这个代码里,bar和try_reset耦合性实在是太强了,强到甚至我调换一下try_reset的调用顺序,都会导致整个逻辑出错。这次出的Bug中有好几个都是这种原因,原始逻辑是1年前写的,上线前调整了一下,结果没测到,然后就出问题了。
我仔细回忆了一下,这种强耦合的实现方式,是我最近两年才开始出现的情况。
之所以会出现这种情况,一方面是因为代码估算做多了之后,我对一条两条指令的开销同样很敏感,以至于很多时候把控不住优化的尺度,把设计弄糟。另一方面,我一直坚持设计正确,不需额外检查这一原则。对于上面的代码,本来设计try_reset就是需要调用方保证,一定在foo.finish为false时才调用,所以try_reset没有理由去检查foo.finish字段。
事实上,这并不是我第一次发现这个问题,以前也发生过几次这种问题。每次发生之后,总是自嘲一句“过早的优化是万恶之源”,然后改正了事。
但是连续几次的Bug, 让我头脑清醒了很多,我觉得我有必要重新审视一下这个问题,这应该不仅仅是过早优化的问题。
这段代码相关的需求大概是这样:如果foo在开始之后,需要完成若干操作,如果在开始之后的规定时间段内没有完成,则重置完成进度,重新完成。
为了“削峰填谷”,我并没有为每一个foo设置一个定时器(即使是固定时间轮),只是在每次获取foo结构时,尝试重置foo的进度。
经过重新审视了整个需求发现,其实“try_reset"的前置条件有两个:一个是没有完成,一个是过期时间。
但是在实现过程中, 把条件1放在了调用方,条件2放在了被调用方(也就是try_reset)。这样try_reset执行结果是否正确,竟然需要调用方来保证。以致于所有调用try_reset的函数,都会与try_reset有强耦合。这其实很像是“契约式编程”,但是除非有语言层面支持,不然只是靠人脑来保证契约前置条件是不靠谱的,所以我个人其实也不是很相信“契约式编程”。
将try_reset函数更改如下,其实可以避免很多Bug, 尤其是多年之后需要修改更时如此:
static viod try_reset(struct foo *f) {
if (f->isfinish == false && f->rsttime < time(NULL)) {
//reset foo fields
}
}
那么上面问题的答案来了,是“过早优化么”?不是。“设计正确不需要检查”这个原则正确么,我依然认为是正确的。因为这种类型的Bug,其根本原因是没有设计好。
根据历史的设计经验,我一般都会有意识的将class/module设计为自成闭环。但是在一个class/module内部函数级实现时,几乎没有仔细思考抽象过,都是实现调用方函数的过程中做的,顺手提取出一个子函数供调用方调用,因此大部分情况下,内部函数和调用方之间都有很强的耦合性。当一个模块过大时,这种耦合性会呈几何倍数增加。
现在回过头来看,其实每一个函数在提取时,都值得仔细抽象来和调用方解偶,即使现在它只有一个调用方。据我过去的经验,你最开始抽象出来的函数,往往会随着业务逻辑的衍变,产生多个调用方。
上面的问题,大部分情况下只是会产生逻辑bug, 一般不太会诱发crash.
据最近几次crash的经验来看,主要原因就是在对于个unordered_map在for循环时,删除了其中的一些元素。比如下面代码:
struct st {
int id;
int progress;
//other fields
};
std::unordered_map<int, struct st> DB;
bool bar(struct st &x)
{
++x.progress;
if (x.progress >= MAX_PROCESS) {
database_delete_by_id(x.id);
DB.erase(x.id);
return false;
}
//process x
return true;
}
void foo(std::vector<int> &l)
{
l.reserve(DB.size())
for (auto &iter:DB) {
auto &x = iter.second;
if (bar(x)) {
l.emplace_back(x.f1);
}
}
}
这种Bug最为讨厌,只要bar函数删除一个元素,就会导致整个迭代器失效,并且不是必崩的。所以这种Bug即使在测试足够的情况下,也很容易逃逸到线上。一般我都会将代码改成如下:
struct st {
int id;
int progress;
//other fields
};
std::unordered_map<int, struct st> DB;
static std::unordered_map<int, struct st>::iterator
check(std::unordered_map<int, struct st>::iterator iter, bool &clear)
{
auto &x = iter->second;
if (x.progress >= MAX_PROCESS) {
database_delete_by_id(x.id);
clear = true;
return DB.erase(iter);
}
clear = false;
return ++iter;
}
static std::unordered_map<int, struct st>::iterator
bar(std::unordered_map<int, struct st>::iterator iter, bool &clear)
{
bool clear;
auto &x = iter->second;
++x.progress;
iter = check(iter, clear);
if (clear == false) {
//process x
}
return iter;
}
void foo(std::vector<int> &l)
{
bool clear;
l.reserve(DB.size())
for (auto iter = DB.begin(); iter != DB.end(); ) {
bar(iter, clear);
if (clear == false) {
l.emplace_back(x.f1);
}
}
}
事实上,我个人很不喜欢这种改动,一股bad taste扑面而来。我也很不喜欢unordered_map的这个限制,他会让for循环方和被调用方强耦合在一起。
for循环方必须保证被调用方不会删除unordered_map中的元素才可以调用。即使是第二种写法,也有一种浓浓的耦合味道。
最近重新审视这种代码时,我发现也许我们可以使用“变换式编程(《程序员修炼之道(第二版)》P149)来解决这种问题。
struct st {
int id;
int progress;
//other fields
};
std::unordered_map<int, struct st> DB;
static std::unordered_map<int, struct st>::iterator
check(std::unordered_map<int, struct st>::iterator iter)
{
auto &x = iter->second;
if (x.progress >= MAX_PROCESS) {
database_delete_by_id(x.id);
return DB.erase(iter);
} else {
return ++iter;
}
}
static void
batch_check()
{
auto iter = DB.begin();
while (iter != DB.end()) {
iter = check(iter);
}
}
static void
batch_process()
{
for (auto &iter:DB)
++iter->second.progress;
}
void foo(std::vector<int> &l)
{
bool clear;
l.reserve(DB.size())
batch_process();
batch_check();
for (auto &iter:DB) {
l.emplace_back(iter.second.f1);
}
}
当然这样的改动会增加一点点性能开销,但是我认为这点开销不会成为热点的致因。
在研究上述问题的同时,我也在想,除了良好的抽象之外,我还能为代码的可靠性做些什么。
是的,答案只有一个,为代码写测试代码。
我查了很多关于单元测试理论性的书籍,他们都告诉我,单元测试就是要把被测试的类通过mock类隔离掉,转而只测试这一个类。
但是游戏服务器的业务太复杂了,以至于会有少量循环引用的情况。这样我实现的mock类到最后,除了可以设置假数据之外,几乎和真实的类的功能一模一样了。
而且我看到书上的很多例子,为了可测试性,会对被测试类的接口加以修改。这种侵入式的测试,我很难适应。
而且游戏服务器还有三大不可控因素: 时间
,配表
,数据库
。
时间
– 在业务逻辑中,有大量时间相关的逻辑,比如多久回复多少资源, 执行一次操作后有多久的CD。与此相对的是,测试代码需要尽可能快的跑完。
配表
– 在业务逻辑中,有大量逻辑是根据配置来,因此测试代码并不能写死,需要根据配置自适应。而且升级配表之后,很难保证测试代码的正确性。
数据库
– 在业务逻辑中,玩家所有的操作造成的影响都会被存入数据库,因此测试不具有可重复性。
其实我很早以前就想为我写过的业务逻辑编写测试代码,但是基于以上种种原因,尝试了几次均以失败而告终。
由于最近出Bug的概率太高了,我下定决心想去解决游戏服务器的单元测式。
即然书上的理论在我这里行不通,那么我去看一些开源项目对单元测试是如何落地的。
我分别考察了Lua和Redis,发现他们都没有做大量侵入式的单元测试。而是通过他们“特有的方式”来触发相关的代码执行。
比如Lua是通过执行Lua代码来测试Lua虚拟机的各种角落,而Redis则是通过网络协议来测试。
我研究了一下这种方式,发现对游戏服务器同样可行。
其实我们根本不需要mock类。理论上我们可以通过网络协议来操作模块处理指定状态。然后再发协议操作某个特定模块,以验证这个模块的正确性。只要我们为服务器的每一个模块编写一个对应的测试操作模块
,就可以大幅度减少测试代码, 大大提高了测试的可行性。
比如account操作模块提供一个account.eusure_create_role_whith_money(name, passwd, money)来保证创建一个名字为name, 密码为passwd, 初始金币为money的账号。
解决了mock类的问题之后,再来看看时间
问题。
为了让测试代码尽可能快的运行,我们只有一种选择,那就是调时间。但是频繁去调开发机的时间,会造成一些很麻烦的后果,而且每个单元测试之后,都要把时间调回来。
好在,当时为了减少time()函数造成的系统调用,我封装了一个time.cpp静态类,所有的时间都是通过time::get()函数来获取的。我设计了一条GM指令,可以调整时间偏移量。为了避免测试相关的GM指令逃逸到线上去。我在本地Makefile加入了一个TEST宏,以确保只有test版本才会有这些指令。如此,时间的问题解决了。事实上不仅仅是调时间,我还加了少部分用于控制特殊逻辑的GM开关,以方便测试代码执行(就像Lua同样在源码里留了一些桩点用于测试使用),这些控制逻辑同样是被TEST控制。
配置表
的问题,咋一看很麻烦,但是静下心来想想,其实我们的代码依赖的并不是表中的内容。而是表结构。所以在写完测试代码之后,除非功能有变化,不然根本没有理由去更改表的内容。惟一麻烦的是,有时候我们需要通过改表来隔绝其他模块造成的干扰。这种情况下,我们就需要为这引单元测试代码定制一份配置表出来。这也意味着我们的单元测试框架需要有,能为不同单元测试代码指定不同配置的能力。
数据库
的问题,其实在我们编写测试操作模块
时就已经解决了。我们只需要像Redis一样,在执行每个单元测试之前,把所有测试进程杀死,并且清空数据库即可。每个单元测试自己负责构造自己所需要的数据。由于我们的测试操作模块
提供的都是很上层的操作。因此创建数据部分,并不会花费太多代码。
基于以上事实,我实现了一个简易的单元测试框架,并配合valgrindt和GCC的ASan,可以同时进行逻辑测试和内存问题测试。
这个框架有部分项目相关性,而且代码并不算太多,因此并没有开源。
ps. 在编辑测试框架过程中,发现一个有意思的问题。当可执行程序在运行过程中,替换其所依赖的so文件会造成进程崩溃。研究了一下发现,这是因为so文件的代码部分是通过mmap到进程内存空间直接执行的。而mmap的特性就是,对映射内存的修改,直接反映到文件,对文件的修改直接反应到mmap的进程内存地址。而cp命的执行步骤一般是,将文件大小截断为0,然后再写入新文件内容。当文件大小截断为0这个步骤,一下子就把so的代码部分破块了,崩溃也就成了必然。