再次吐槽审阅代码发现的问题

bihai
new half life 09月17日 字数 1299

有个很努力的组员提交了代码,经过多次讨论后,如下

Worker::Worker(std::string file)

Worker::DoWork(){

Controller c(config);

c.StartThread();

Info info(file_);

ImportantCall(info.Start(c.loop()));

}

这里的具体技术是, Controller负责启动一个线程,这个线程给info启动,然后通过ImportantCall把启动后的info传给库。库里的代码呼叫info可以通过这个线程来响应。可以看做是通过网络通讯的,不是直接通过堆栈的调用。DoWork后,info自动消失,线程消失,完美。

有个土人给了意见,为什么需要另一个线程?如果真需要,要在main里面启动,然后把c.loop传进Worker的构造函数。

这里面有一些调试的发现,如果不启动新的线程,而是用当前线程的loop,info.Start(current.loop()),会死锁。因为ImportantCall是阻塞的。这个很努力的组员的代码很简单,很美好,可是这个意见要求很不正常。原来的代码实现了封装,需要的线程自动启动,自动结束。意见要求却从main启动。

而且吧,我已经发现了,如果把当前线程的loop传给ImportantCall会死锁的。原来的代码无论如何不会死锁,因为内部开始新的线程,用新的loop。而意见要求调用者准备好线程,并传进loop

Worker::Worker(std::string file, Loop loop):file_(file), new_loop_(loop)

Worker::DoWork(){

Controller c(config);

c.StartThread();

Info info(file_);

ImportantCall(info.Start(new_loop_));

}

这样的话,调用者就必须知道要实现一个线程,要传进新线程的loop,这比原来糟糕了。这需要调用者学新的知识才会怎么用,而且有发生错误的可能。

4 个回复
ziqin
子青|会挽雕弓如满月|西北望|射天狼 09月18日

讲真,只能说大概猜到你的意思。

1. 看使用环境,如果是低频偶尔使用,随便来

2. 如果是性能相关的部分,肯定得外部传线程进去。线程怎么传是写库的人的是,copy/move/sharedptr要看你们的库怎么涉及。

而且我个人意见是

1. 如果本身就是rpc框架的设计,那本来就应该是block call,如果是异步设计,你们这个框架设计本身就过时了。本身就应该是类似threadpool.posttask之类的基于task的设计,而不是把线程传给类

bihai
new half life 09月18日

你说的很有道理。我们这个是超低频使用,仅此一次。

那个“土人”看来不是很土,他们的代码线程用了十几次吧,所以在外面设好了传进来,类似pool。所以他会这样建议。其实他们的代码不需要两个线程,而且为了避免多线程的问题加了很多保护措施。

我准备把我参考的这个代码在几年后公开。其实现在是开源的。但是写得略微有些不足。

【 在 ziqin 的大作中提到: 】

: 讲真,只能说大概猜到你的意思。

: 1. 看使用环境,如果是低频偶尔使用,随便来

: 2. 如果是性能相关的部分,肯定得外部传线程进去。线程怎么传是写库的人的是,copy/move/sharedptr要看你们的库怎么涉及。

: ...................

ziqin
子青|会挽雕弓如满月|西北望|射天狼 09月18日

其实就是asio的设计吧。iocontext.run可以让用户来选择由哪个thread来跑。彻底把task/scheduler/worker去耦合了

bihai
new half life 09月21日

主要是异步,也支持同步的多线程,但是说是不一定好用。以前出现过崩溃,就是一个类在两个线程调度中用了,就出错了。改成PostTask就好了。

这次这个项目参考了一个以前的实现,别人是用两个线程,本来的线程是客户端,新的线程产生了本地服务器,客户端把本地服务器的信息发给远程服务器,远程再访问本地服务器,所以两个线程基本互不干扰。

但是“土人”非说要避免多线程。后来发现确实可以改。我们的模式是一步一步完成小任务,等待的方法是使用一种promise。同步做很容易,在里面等待线程结束即可。异步的话,我估摸着有一些机制就是可以唤醒消息循环,所以访问本地服务器的消息和最后结束的消息在消息循环里面处理了。让年轻人试试,到时候我也学习一下

【 在 ziqin 的大作中提到: 】

: 其实就是asio的设计吧。iocontext.run可以让用户来选择由哪个thread来跑。彻底把task/scheduler/worker去耦合了