Multi-threading is difficult to get right even for the seasoned developer, a common source of “mostly works” type of program. Try to spot the bug in the example below.

Disclaimer

DO NOT USE THE CODE BELOW BECAUSE IT CONTAINS BUGS. IT IS SIMPLY AN EXERCISE FOR FINDING THE BUGS.

The fair-weather coder pitch

Let’s try to explain the intent for the code below.

std::thread is a weird beast. Most classes perform cleanup on destructor. However std::thread requires you to perform cleanup, in this case to ensure that the thread is told to end before the destructor of std::thread , or else it terminates the process.

peridic_thread is intended as a reusable class that wraps a std::thread to address the issue above for the case where some action needs to be performed periodically. Using a carefully balanced combination of std::mutex and std::condition_variable, it calls periodically a virtual function on_fire and orchestrates the thread startup (in constructor) and shutdown (and destructor).

some_peridic_thread is an example of using periodic_thread. It derives from it and configures the interval to be every 2 seconds. It also implements on_fire.

main instantiates a some_periodic_thread and then it waits a bit (5 seconds). That’s just enough time for the on_fire function to be called twice.

The reality

The reality is that the code below has a bug that leads to undefined behaviour which weirdly in this case means that in practice it almost always works as expected. Unlike the bugs that are easy to reproduce and one can use a debugger to step through, this kind of bug usually requires careful code review to identify.

Try to spot the bug.

Full source code

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
#include <chrono>
#include <condition_variable>
#include <functional>
#include <iostream>
#include <mutex>
#include <thread>

namespace
{
  class periodic_thread
  {
    std::chrono::seconds sleep_duration_;
    bool must_stop_;
    std::mutex mutex_;
    std::condition_variable condition_variable_;
    std::thread thread_;

  public:
    explicit periodic_thread(
      const std::chrono::seconds & sleep_duration
      ) :
      sleep_duration_{ sleep_duration },
      must_stop_{ false },
      thread_{ [this](){ this->run(); } }
    {
    }

    virtual ~periodic_thread()
    {
      {
        std::scoped_lock lock(mutex_);
        must_stop_ = true;
      }
      condition_variable_.notify_one();
      thread_.join();
    }

    virtual void on_fire() = 0;

  private:
    void run() noexcept
    {
      std::unique_lock lock(mutex_);

      while ( ! must_stop_)
      {
        condition_variable_.wait_for(lock, sleep_duration_);
        if (must_stop_)
        {
          return;
        }

        on_fire();
      }
    }
  };

  class some_periodic_thread :
    private periodic_thread
  {
  public:
    some_periodic_thread() :
      periodic_thread(std::chrono::seconds(2))
    {
    }

  private:
    void on_fire() override
    {
      std::cout << "Fire" << std::endl;
    }

  };

  void wait_a_bit()
  {
    std::this_thread::sleep_for(
      std::chrono::seconds(5));
  }
} // anonymous namespace

int main()
{
  {
    some_periodic_thread p;
    wait_a_bit();
  }
  std::cout << "Done" << std::endl;
}

// Compile with:
//   g++ -std=c++17 -pthread main.cpp && ./a.out
// Prints:
//   Fire
//   Fire
//   Done