NodeImpl析构低概率死锁

[zhongleihe]

NodeImpl是Node的一个成员，NodeImpl本身是智能指针，而引用NodeImpl的地方有下面几个：
（1）Node（使用者控制析构）
（2）FollowerStableClosure、ConfigurationChangeDone
（3）其他
考虑下面执行顺序：
（1）Node析构时调用_impl->Release()，调用完后_impl的引用计数为1，不会调用_impl的析构函数
（2）这个时候log_manager的_disk_queue中还有个FollowerStableClosure的请求，在FollowerStableClosure析构函数如下：
FollowerStableClosure() {
if (_node) {
_node->Release();//此时_impl的引用计数为0，调用NodeImpl的析构
}
}
（3）在NodeImpl的析构用会删除_log_manager
if (_log_manager) {
delete _log_manager;
_log_manager = NULL;
}
而在LogManager中会调用stop_disk_thread
int LogManager::stop_disk_thread() {
bthread::execution_queue_stop(_disk_queue);//会向_disk_queue中插入stop消息
return bthread::execution_queue_join(_disk_queue);//等待执行到stop消息
}
上面的时序会导致相互依赖而导致死锁。FollowerStableClosure消息的消费依赖于NodeImpl的析构，而NodeImpl的析构有依赖于FollowerStableClosure被消费

[PFZheng]

“FollowerStableClosure消息的消费依赖于NodeImpl的析构” 这一条不成立吧。FollowerStableClosure 的消费取决于 disk thread 的推进，这个是独立工作的

[zhongleihe]

“FollowerStableClosure消息的消费依赖于NodeImpl的析构” 这一条不成立吧。FollowerStableClosure 的消费取决于 disk thread 的推进，这个是独立工作的

问题出在_disk_queue中最后一条没有消费的消息是FollowerStableClosure，而FollowerStableClosure中持有NodeImpl的引用计数，并且在LogManager::disk_thread调用done->Run()时会调用NodeImpl的Release，这时候正好引用计数为0，调用NodeImpl的析构。所以disk_thread的协程想要消费掉FollowerStableClosure需要NodeImpl析构完。

[PFZheng]

“FollowerStableClosure消息的消费依赖于NodeImpl的析构” 这一条不成立吧。FollowerStableClosure 的消费取决于 disk thread 的推进，这个是独立工作的

问题出在_disk_queue中最后一条没有消费的消息是FollowerStableClosure，而FollowerStableClosure中持有NodeImpl的引用计数，并且在LogManager::disk_thread调用done->Run()时会调用NodeImpl的Release，这时候正好引用计数为0，调用NodeImpl的析构。所以disk_thread的协程想要消费掉FollowerStableClosure需要NodeImpl析构完。

我大概明白你的意思了，这里说的case是指，在 disk thread 里 join了disk thread 自身的stop吧，因为这个原地 join 的存在，disk thread 本身结束不了

[zhongleihe]

“FollowerStableClosure消息的消费依赖于NodeImpl的析构” 这一条不成立吧。FollowerStableClosure 的消费取决于 disk thread 的推进，这个是独立工作的

问题出在_disk_queue中最后一条没有消费的消息是FollowerStableClosure，而FollowerStableClosure中持有NodeImpl的引用计数，并且在LogManager::disk_thread调用done->Run()时会调用NodeImpl的Release，这时候正好引用计数为0，调用NodeImpl的析构。所以disk_thread的协程想要消费掉FollowerStableClosure需要NodeImpl析构完。

我大概明白你的意思了，这里说的case是指，在 disk thread 里 join了disk thread 自身的stop吧，因为这个原地 join 的存在，disk thread 本身结束不了

是这个意思，概率比较低，在大量析构NodeImpl，并且有数据写入的情况下才会出现

[PFZheng]

“FollowerStableClosure消息的消费依赖于NodeImpl的析构” 这一条不成立吧。FollowerStableClosure 的消费取决于 disk thread 的推进，这个是独立工作的

问题出在_disk_queue中最后一条没有消费的消息是FollowerStableClosure，而FollowerStableClosure中持有NodeImpl的引用计数，并且在LogManager::disk_thread调用done->Run()时会调用NodeImpl的Release，这时候正好引用计数为0，调用NodeImpl的析构。所以disk_thread的协程想要消费掉FollowerStableClosure需要NodeImpl析构完。

我大概明白你的意思了，这里说的case是指，在 disk thread 里 join了disk thread 自身的stop吧，因为这个原地 join 的存在，disk thread 本身结束不了

是这个意思，概率比较低，在大量析构NodeImpl，并且有数据写入的情况下才会出现

嗯，我们看下这个问题怎么fix

[zhongleihe]

“FollowerStableClosure消息的消费依赖于NodeImpl的析构” 这一条不成立吧。FollowerStableClosure 的消费取决于 disk thread 的推进，这个是独立工作的

问题出在_disk_queue中最后一条没有消费的消息是FollowerStableClosure，而FollowerStableClosure中持有NodeImpl的引用计数，并且在LogManager::disk_thread调用done->Run()时会调用NodeImpl的Release，这时候正好引用计数为0，调用NodeImpl的析构。所以disk_thread的协程想要消费掉FollowerStableClosure需要NodeImpl析构完。

我大概明白你的意思了，这里说的case是指，在 disk thread 里 join了disk thread 自身的stop吧，因为这个原地 join 的存在，disk thread 本身结束不了

是这个意思，概率比较低，在大量析构NodeImpl，并且有数据写入的情况下才会出现

嗯，我们看下这个问题怎么fix

我们这边有几个方案，不过都感觉改的比较丑。有结论了辛苦通知下，我们这边发生的概率有点高

[PFZheng]

“FollowerStableClosure消息的消费依赖于NodeImpl的析构” 这一条不成立吧。FollowerStableClosure 的消费取决于 disk thread 的推进，这个是独立工作的

问题出在_disk_queue中最后一条没有消费的消息是FollowerStableClosure，而FollowerStableClosure中持有NodeImpl的引用计数，并且在LogManager::disk_thread调用done->Run()时会调用NodeImpl的Release，这时候正好引用计数为0，调用NodeImpl的析构。所以disk_thread的协程想要消费掉FollowerStableClosure需要NodeImpl析构完。

我大概明白你的意思了，这里说的case是指，在 disk thread 里 join了disk thread 自身的stop吧，因为这个原地 join 的存在，disk thread 本身结束不了

是这个意思，概率比较低，在大量析构NodeImpl，并且有数据写入的情况下才会出现

嗯，我们看下这个问题怎么fix

我们这边有几个方案，不过都感觉改的比较丑。有结论了辛苦通知下，我们这边发生的概率有点高

嗯，好的，这个 case 确实有些麻烦

[PFZheng]

“FollowerStableClosure消息的消费依赖于NodeImpl的析构” 这一条不成立吧。FollowerStableClosure 的消费取决于 disk thread 的推进，这个是独立工作的

问题出在_disk_queue中最后一条没有消费的消息是FollowerStableClosure，而FollowerStableClosure中持有NodeImpl的引用计数，并且在LogManager::disk_thread调用done->Run()时会调用NodeImpl的Release，这时候正好引用计数为0，调用NodeImpl的析构。所以disk_thread的协程想要消费掉FollowerStableClosure需要NodeImpl析构完。

我大概明白你的意思了，这里说的case是指，在 disk thread 里 join了disk thread 自身的stop吧，因为这个原地 join 的存在，disk thread 本身结束不了

是这个意思，概率比较低，在大量析构NodeImpl，并且有数据写入的情况下才会出现

嗯，我们看下这个问题怎么fix

我们这边有几个方案，不过都感觉改的比较丑。有结论了辛苦通知下，我们这边发生的概率有点高

嗯，好的，这个 case 确实有些麻烦

一个可行的改法是把 disk thread 的 stop 和 join 的过程移到 NodeImpl shutdown 和 join 接口里，这样可以保证最后一次 ref 不会发生在 disk thread 里

[zhongleihe]

“FollowerStableClosure消息的消费依赖于NodeImpl的析构” 这一条不成立吧。FollowerStableClosure 的消费取决于 disk thread 的推进，这个是独立工作的

问题出在_disk_queue中最后一条没有消费的消息是FollowerStableClosure，而FollowerStableClosure中持有NodeImpl的引用计数，并且在LogManager::disk_thread调用done->Run()时会调用NodeImpl的Release，这时候正好引用计数为0，调用NodeImpl的析构。所以disk_thread的协程想要消费掉FollowerStableClosure需要NodeImpl析构完。

我大概明白你的意思了，这里说的case是指，在 disk thread 里 join了disk thread 自身的stop吧，因为这个原地 join 的存在，disk thread 本身结束不了

是这个意思，概率比较低，在大量析构NodeImpl，并且有数据写入的情况下才会出现

嗯，我们看下这个问题怎么fix

我们这边有几个方案，不过都感觉改的比较丑。有结论了辛苦通知下，我们这边发生的概率有点高

嗯，好的，这个 case 确实有些麻烦

一个可行的改法是把 disk thread 的 stop 和 join 的过程移到 NodeImpl shutdown 和 join 接口里，这样可以保证最后一次 ref 不会发生在 disk thread 里

我们这样改过，有问题。因为_apply_queue的stop和join是在NodeImpl的析构中做的，如果把disk_thread的stop和join提前到NodeImpl的shutdown和join，有可能disk_queue stop了，但_apply_queue没有stop，还会有请求要push到disk_queue中，下面的check过不了：
done->_entries.swap(*entries);
int ret = bthread::execution_queue_execute(_disk_queue, done);
CHECK_EQ(0, ret) << "execq execute failed, ret: " << ret << " err: " << berror();
wakeup_all_waiter(lck);
现在我们想到两种方式，都不太好，可以讨论下：
（1）把disk_queue的done的析构另开一个协程去处理，缺点是这点事开个协程去处理有点中，大部分情况下done的析构很轻量级
（2）把NodeImpl的析构放在单独的协程中去处理，这样从性能上比较好。但代码看着有点奇怪。

[PFZheng]

“FollowerStableClosure消息的消费依赖于NodeImpl的析构” 这一条不成立吧。FollowerStableClosure 的消费取决于 disk thread 的推进，这个是独立工作的

问题出在_disk_queue中最后一条没有消费的消息是FollowerStableClosure，而FollowerStableClosure中持有NodeImpl的引用计数，并且在LogManager::disk_thread调用done->Run()时会调用NodeImpl的Release，这时候正好引用计数为0，调用NodeImpl的析构。所以disk_thread的协程想要消费掉FollowerStableClosure需要NodeImpl析构完。

我大概明白你的意思了，这里说的case是指，在 disk thread 里 join了disk thread 自身的stop吧，因为这个原地 join 的存在，disk thread 本身结束不了

是这个意思，概率比较低，在大量析构NodeImpl，并且有数据写入的情况下才会出现

嗯，我们看下这个问题怎么fix

我们这边有几个方案，不过都感觉改的比较丑。有结论了辛苦通知下，我们这边发生的概率有点高

嗯，好的，这个 case 确实有些麻烦

一个可行的改法是把 disk thread 的 stop 和 join 的过程移到 NodeImpl shutdown 和 join 接口里，这样可以保证最后一次 ref 不会发生在 disk thread 里

我们这样改过，有问题。因为_apply_queue的stop和join是在NodeImpl的析构中做的，如果把disk_thread的stop和join提前到NodeImpl的shutdown和join，有可能disk_queue stop了，但_apply_queue没有stop，还会有请求要push到disk_queue中，下面的check过不了：
done->_entries.swap(*entries);
int ret = bthread::execution_queue_execute(_disk_queue, done);
CHECK_EQ(0, ret) << "execq execute failed, ret: " << ret << " err: " << berror();
wakeup_all_waiter(lck);
现在我们想到两种方式，都不太好，可以讨论下：
（1）把disk_queue的done的析构另开一个协程去处理，缺点是这点事开个协程去处理有点中，大部分情况下done的析构很轻量级
（2）把NodeImpl的析构放在单独的协程中去处理，这样从性能上比较好。但代码看着有点奇怪。

apply_queue 这里 _state 的检查应该过不了，shutdown 后这里的状态不在是 leader 了。flush版本的几个id获取函数是可能有问题的，不过 queue stop 的话，原地取值就是准确的。

[PFZheng]

还有一个思路，在你说的 (1) 的基础上做一下改进，stop 的时候插入一个 barrier stop 事件，在这个事件之后的请求 done->Run() 新启协程执行，barrier 的状态只有 disk thread 会关心，这里不需要加任何竞争保护，stop之后的开销退化应该也是可以接受的，新事件的插入是个小概率事件

[PFZheng]

@zhongleihe review 一下这个pull request，看看有没有问题

[zhongleihe]

@zhongleihe review 一下这个pull request，看看有没有问题

没啥问题，我们也这样修改下，正好验证下。我们这边的复现概率很高

[PFZheng]

@zhongleihe review 一下这个pull request，看看有没有问题

没啥问题，我们也这样修改下，正好验证下。我们这边的复现概率很高

嗯，可以

[GOGOYAO]

我们也遇到了shutdown的时候，死锁的问题。我们的现象是on_shutdown一直不对调用，join就一直无法退出了