CodingHanYa / workspace

workspace是基于C++11的轻量级异步执行框架,支持:通用任务异步并发执行、优先级任务调度、自适应动态线程池、高效静态线程池、异常处理机制等。
Apache License 2.0
916 stars 136 forks source link

[bug?] 可能不需要条件变量的互斥锁保护 #13

Closed LEON-REIN closed 1 year ago

LEON-REIN commented 1 year ago

代码中有多处在使用条件变量的 .notify_all().notify_one() 前用了锁保护. 但是根据:

  1. stackoverflow
  2. cppreference--注意--最后一句

来看, 似乎不需要加锁?

在代码中的位置:

  1. https://github.com/CodingHanYa/Hipe/blob/main/src/dynamic_pond.h#L106
  2. https://github.com/CodingHanYa/Hipe/blob/main/src/dynamic_pond.h#L229
  3. https://github.com/CodingHanYa/Hipe/blob/main/src/dynamic_pond.h#L265
  4. https://github.com/CodingHanYa/Hipe/blob/main/src/header.h#L79
CodingHanYa commented 1 year ago

在修改通知变量时加锁或者在notify前加锁都可以避免没有通知到异步线程的情况。而后者某些情况下其实更加简便,不容易出错。https://github.com/CodingHanYa/Hipe/blob/main/src/dynamic_pond.h#L265,就像在这里我们势必要解锁后执行任务,但我们可能会因为忘记给total_tasks加锁而导致出现没有通知到异步线程的情况。

LEON-REIN commented 1 year ago

在 GCC 的 condition_variable 实现中两个 notify 函数中已经有了加锁~ 似乎用的类内自己的 mutex:

    void
    notify_one() noexcept
    {
      lock_guard<mutex> __lock(*_M_mutex);
      _M_cond.notify_one();
    }

    void
    notify_all() noexcept
    {
      lock_guard<mutex> __lock(*_M_mutex);
      _M_cond.notify_all();
    }
CodingHanYa commented 1 year ago

这个是保护notify本身的锁,不能和保护wait的锁互斥

LEON-REIN commented 1 year ago

好的! 感谢回复~