From 757737e86d9727aba1f9039def1aee8b0e8112f1 Mon Sep 17 00:00:00 2001 From: Boris Smidt Date: Sun, 24 Apr 2016 14:50:50 +0200 Subject: [PATCH 1/2] changed tabs into 4 spaces --- CppCoreGuidelines.md | 70 ++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index d278e5e..0efb317 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -10786,13 +10786,13 @@ A `thread` that has not been `detach()`ed when it is destroyed terminates the pr void f() { std::cout << "Hello "; } struct F { - void operator()() { std::cout << "parallel world "; } + void operator()() { std::cout << "parallel world "; } }; int main() { - std::thread t1{f}; // f() executes in separate thread - std::thread t2{F()}; // F()() executes in separate thread + std::thread t1{f}; // f() executes in separate thread + std::thread t2{F()}; // F()() executes in separate thread } // spot the bugs ##### Example @@ -10800,13 +10800,13 @@ A `thread` that has not been `detach()`ed when it is destroyed terminates the pr void f() { std::cout << "Hello "; } struct F { - void operator()() { std::cout << "parallel world "; } + void operator()() { std::cout << "parallel world "; } }; int main() { - std::thread t1{f}; // f() executes in separate thread - std::thread t2{F()}; // F()() executes in separate thread + std::thread t1{f}; // f() executes in separate thread + std::thread t2{F()}; // F()() executes in separate thread t1.join(); t2.join(); @@ -10992,20 +10992,20 @@ A `wait` without a condition can miss a wakeup or wake up simply to find that th void thread1() { - while (true) { - // do some work ... - std::unique_lock lock(mx); - cv.notify_one(); // wake other thread - } + while (true) { + // do some work ... + std::unique_lock lock(mx); + cv.notify_one(); // wake other thread + } } void thread2() { - while (true) { - std::unique_lock lock(mx); - cv.wait(lock); // might block forever - // do work ... - } + while (true) { + std::unique_lock lock(mx); + cv.wait(lock); // might block forever + // do work ... + } } Here, if some other `thread` consumes `thread1`'s notification, `thread2` can wait forever. @@ -11015,30 +11015,30 @@ Here, if some other `thread` consumes `thread1`'s notification, `thread2` can wa template class Sync_queue { public: - void put(const T& val); - void put(T&& val); - void get(T& val); + void put(const T& val); + void put(T&& val); + void get(T& val); private: - mutex mtx; - condition_variable cond; // this controls access - list q; + mutex mtx; + condition_variable cond; // this controls access + list q; }; template void Sync_queue::put(const T& val) { - lock_guard lck(mtx); - q.push_back(val); - cond.notify_one(); + lock_guard lck(mtx); + q.push_back(val); + cond.notify_one(); } template void Sync_queue::get(T& val) { - unique_lock lck(mtx); - cond.wait(lck,[this]{ return !q.empty(); }); // prevent spurious wakeup - val=q.front(); - q.pop_front(); + unique_lock lck(mtx); + cond.wait(lck,[this]{ return !q.empty(); }); // prevent spurious wakeup + val=q.front(); + q.pop_front(); } Now if the queue is empty when a thread executing `get()` wakes up (e.g., because another thread has gotton to `get()` before it), @@ -11251,15 +11251,15 @@ It's error-prone and requires expert level knowledge of language features, machi ##### Example, bad - extern atomic head; // the shared head of a linked list + extern atomic head; // the shared head of a linked list - Link* nh = new Link(data,nullptr); // make a link ready for insertion - Link* h = head.load(); // read the shared head of the list + Link* nh = new Link(data,nullptr); // make a link ready for insertion + Link* h = head.load(); // read the shared head of the list do { - if (h->data<=data) break; // if so, insert elsewhere - nh->next = h; // next element is the previous head - } while (!head.compare_exchange_weak(h,nh)); // write nh to head or to h + if (h->data<=data) break; // if so, insert elsewhere + nh->next = h; // next element is the previous head + } while (!head.compare_exchange_weak(h,nh)); // write nh to head or to h Spot the bug. It would be really hard to find through testing. From 122ce835098a26e87c84abecdf2fbf0e3c03bdf3 Mon Sep 17 00:00:00 2001 From: Boris Smidt Date: Sun, 24 Apr 2016 17:29:02 +0200 Subject: [PATCH 2/2] fixed markdown style errors --- CppCoreGuidelines.md | 128 ++++++++++++++++++++++--------------------- 1 file changed, 66 insertions(+), 62 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 0efb317..b077d8e 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -5746,7 +5746,7 @@ Such as on an ABI (link) boundary. class D2 : public Device { // ... different data ... - + void write(span outbuf) override; void read(span inbuf) override; }; @@ -6815,6 +6815,7 @@ Union rule summary: ##### Example + ??? ##### Enforcement @@ -6825,6 +6826,7 @@ Union rule summary: ##### Reason + Naked unions are a source of type errors. **Alternative**: Wrap them in a class together with a type field. @@ -6835,6 +6837,8 @@ Naked unions are a source of type errors. ??? + + ##### Enforcement ??? @@ -10226,7 +10230,7 @@ However, thanks to the magic of cut-and-paste, code fragments can turn up in une static double cached_x = 0.0; static double cached_result = COMPUTATION_OF_ZERO; double result; - + if (cached_x == x) return cached_result; result = computation(x); @@ -10282,9 +10286,9 @@ including: `id` plus one. * Thread A and B load `id` and increment it simultaneously. They both get the same ID. - + Local static variables are a common source of data races. - + ##### Example, bad: void f(fstream& fs, regex pat) @@ -10298,7 +10302,7 @@ Local static variables are a common source of data races. auto h2 = async([&]{ return find_all(buf,sz,pat); }); // span a task to find matches // ... } - + Here, we have a (nasty) data race on the elements of `buf` (`sort` will both read and write). All data races are nasty. Here, we managed to get a data race on data on the stack. @@ -10307,7 +10311,7 @@ Not all data races are as easy to spot as this one. ##### Example, bad: // code not controlled by a lock - + unsigned val; if (val < 5) { @@ -10322,7 +10326,7 @@ Not all data races are as easy to spot as this one. } Now, a compiler that does not know that `val` can change will most likely implement that `switch` using a jump table with five entries. -Then, a `val` outside the [0..4] range will cause a jump to an address that could be anywhere in the program, and execution would proceed there. +Then, a `val` outside the `[0..4]` range will cause a jump to an address that could be anywhere in the program, and execution would proceed there. Really, "all bets are off" if you get a data race. Actually, it can be worse still: by looking at the generated code you may be able to determine where the stray jump will go for a given value; this can be a security risk. @@ -10354,13 +10358,13 @@ The less sharing you do, the less chance you have to wait on a lock (so performa Graph validate(const vector&); Image validate(const vector&); // ... - + void process_readings(istream& socket1) { vector surface_readings; socket1 >> surface_readings; if (!socket1) throw Bad_input{}; - + auto h1 = async([&] { if (!validate(surface_readings) throw Invalide_data{}; }); auto h2 = async([&] { return temparature_gradiants(surface_readings); }); auto h3 = async([&] { return altitude_map(surface_readings); }); @@ -10370,7 +10374,7 @@ The less sharing you do, the less chance you have to wait on a lock (so performa auto v3 = h3.get(); // ... } - + Without those `const`s, we would have to review every asynchroneously invoked function for potential data races on `surface_readings`. ##### Note @@ -10394,7 +10398,7 @@ Application concepts are easier to reason about. ##### Example ??? - + ###### Note With the exception of `async()`, the standard-library facilities are low-level, machine-oriented, threads-and-lock level. @@ -10403,7 +10407,7 @@ This is a potent argument for using higher level, more applications-oriented lib ##### Enforcement -??? +??? ### CP.8 Don't try to use `volatile` for synchronization @@ -10421,7 +10425,7 @@ It simply has nothing to do with concurrency. { if (int n = free_slots--) return &pool[n]; } - + Here we have a problem: This is perfectly good code in a single-threaded program, but have two treads exectute this and there is a race condition on `free_slots` so that two threads might get the same value and `free_slots`. @@ -10501,20 +10505,20 @@ Avoids nasty errors from unreleased locks. ##### Example, bad mutex mtx; - + void do_stuff() { mtx.lock(); // ... do stuff ... mtx.unlock(); } - + Sooner or later, someone will forget the `mtx.unlock()`, place a `return` in the `... do stuff ...`, throw an exception, or something. ##### Example mutex mtx; - + void do_stuff() { unique_lock lck {mtx}; @@ -10539,23 +10543,23 @@ This is asking for deadlock: // thread 1 lock_guard lck1(m1); lock_guard lck2(m2); - + // thread 2 lock_guard lck2(m2); lock_guard lck1(m1); - + Instead, use `lock()`: // thread 1 lock_guard lck1(m1,defer_lock); lock_guard lck2(m2,defer_lock); lock(lck1,lck2); - + // thread 2 lock_guard lck2(m2,defer_lock); lock_guard lck1(m1,defer_lock); lock(lck2,lck1); - + Here, the writers of `thread1` and `thread2` are still not agreeing on the order of the `mutex`es, but order no longer matters. ##### Note @@ -10566,7 +10570,7 @@ In real code, `mutex`es are not always conveniently aquired on consequtive lines I'm really looking forward to be able to write plain lock_guard lck1(m1,defer_lock); - + and have the `mutex` type deduced. ##### Enforcement @@ -10601,7 +10605,7 @@ A common example of the "calling unknown code" problem is a call to a function t Such problem cal often be solved by using a `recursive_mutex`. For example: recursive_mutex my_mutex; - + template void do_something(Action f) { @@ -10610,9 +10614,9 @@ Such problem cal often be solved by using a `recursive_mutex`. For example: f(this); // f will do something to *this // ... } - + If, as it is likely, `f()` invokes operations on `*this`, we must make sure that the object's invariant holds before the call. - + ##### Enforcement * Flag calling a virtual function with a non-recursive `mutex` held @@ -10635,7 +10639,7 @@ If a `thread` joins, we can safely pass pointers to objects in the scope of the // ... } int glob = 33; - + void some_fct(int* p) { int x = 77; @@ -10673,16 +10677,16 @@ If a `thread` is detached, we can safely pass pointers to static and free store *p = 99; // ... } - + int glob = 33; - + void some_fct(int* p) { int x = 77; std::thread t0(f,&x); // bad std::thread t1(f,p); // bad std::thread t2(f,&glob); // OK - auto q = make_unique(99); + auto q = make_unique(99); std::thread t3(f,q.get()); // bad // ... t0.detach(); @@ -10708,7 +10712,7 @@ After that, the usual lifetime and ownership (for global objects) enforcement ap ##### Reason -An `raii_thread` is a thread that joins at the end of its scope. +An `raii_thread` is a thread that joins at the end of its scope. Detatched threads are hard to monitor. @@ -10732,7 +10736,7 @@ Documenting that aids comprehension and helps static analysis. ##### Example void heartbeat(); - + void use() { gsl::detached_thread t1(heartbeat); // obviously need not be joined @@ -10750,7 +10754,7 @@ Flag unconditional `detach` on a plain `thread` ##### Reason `thread`s that are supposed to unconditionally `join` or unconditionally `detach` can be clearly identified as such. -The plain `thread`s should be assumed to use the full generality of `std::thread`. +The plain `thread`s should be assumed to use the full generality of `std::thread`. ##### Example @@ -10761,7 +10765,7 @@ The plain `thread`s should be assumed to use the full generality of `std::thread t->detach(); // ... } - + void use(int n) { thread t { thricky, this, n }; @@ -10778,7 +10782,7 @@ The plain `thread`s should be assumed to use the full generality of `std::thread ### CP.28: Remember to join scoped `thread`s that are not `detach()`ed ##### Reason - + A `thread` that has not been `detach()`ed when it is destroyed terminates the program. ##### Example, bad @@ -10789,9 +10793,9 @@ A `thread` that has not been `detach()`ed when it is destroyed terminates the pr void operator()() { std::cout << "parallel world "; } }; - int main() + int main() { - std::thread t1{f}; // f() executes in separate thread + std::thread t1{f}; // f() executes in separate thread std::thread t2{F()}; // F()() executes in separate thread } // spot the bugs @@ -10803,15 +10807,15 @@ A `thread` that has not been `detach()`ed when it is destroyed terminates the pr void operator()() { std::cout << "parallel world "; } }; - int main() + int main() { - std::thread t1{f}; // f() executes in separate thread + std::thread t1{f}; // f() executes in separate thread std::thread t2{F()}; // F()() executes in separate thread - + t1.join(); t2.join(); } // one bad bug left - + ??? Is `cout` synchronized? ##### Enforcement @@ -10835,7 +10839,7 @@ In general, you cannot know whether a non-`raii_thread` will outlife your thread // ... t0.detach(); } - + The detach` may not be so easy to spot. Use a `raii_thread` or don't pass the pointer. @@ -10863,13 +10867,13 @@ Defining "small amount" precisely and is impossible. string modify1(string); void modify2(shared_ptr work; - + void master(istream& is) { for (Message m; is>>m; ) work.put(n); } - + void worker() { for (Message m; m=work.get(); ) { // process } } - + void workers() // set up worker threads (specifically 4 worker threads) { raii_thread w1 {worker}; @@ -10968,7 +10972,7 @@ Instead, we could have a set of pre-created worker threads processing the messag raii_thread w3 {worker}; raii_thread w4 {worker}; } - + ###### Note If you system has a good thread pool, use it. @@ -10990,11 +10994,11 @@ A `wait` without a condition can miss a wakeup or wake up simply to find that th std::condition_variable cv; std::mutex mx; - void thread1() + void thread1() { while (true) { // do some work ... - std::unique_lock lock(mx); + std::unique_lock lock(mx); cv.notify_one(); // wake other thread } } @@ -11002,11 +11006,11 @@ A `wait` without a condition can miss a wakeup or wake up simply to find that th void thread2() { while (true) { - std::unique_lock lock(mx); + std::unique_lock lock(mx); cv.wait(lock); // might block forever // do work ... } - } + } Here, if some other `thread` consumes `thread1`'s notification, `thread2` can wait forever. @@ -11065,7 +11069,7 @@ and `thread` suspection and resumption are expensive. do1(); // transaction: needs locking do2(); // cleanup: does not need locking } - + Here, we are holding the lock for longer than necessary: We should not have taken the lock before we needed it and should have released it again before starting the cleanup. We could rewrite this to @@ -11078,7 +11082,7 @@ We could rewrite this to my_lock.unluck(); do2(); // cleanup: does not need locking } - + But that compromises safety and violates the [use RAII](#Rconc-raii) rule. Instead, add a block for the critical section: @@ -11091,7 +11095,7 @@ Instead, add a block for the critical section: } do2(); // cleanup: does not need locking } - + ##### Enforcement Impossible in general. @@ -11109,7 +11113,7 @@ An unnamed local objects is a temporary that immediately goes out of scope. unique_lock(m1); lock_guard {m2}; lock(m1,m2); - + This looks innocent enough, but it isn't. ##### Enforcement @@ -11130,7 +11134,7 @@ It should be obvious to a reader that the data is to be guarded and how. std::mutex m; // take this mutex before accessing other members // ... }; - + ##### Enforcement ??? Possible? @@ -11255,12 +11259,12 @@ It's error-prone and requires expert level knowledge of language features, machi Link* nh = new Link(data,nullptr); // make a link ready for insertion Link* h = head.load(); // read the shared head of the list - + do { if (h->data<=data) break; // if so, insert elsewhere nh->next = h; // next element is the previous head } while (!head.compare_exchange_weak(h,nh)); // write nh to head or to h - + Spot the bug. It would be really hard to find through testing. Read up on the ABA problem. @@ -11311,7 +11315,7 @@ Become an expert before shipping lock-free code for others to use. * Mark Batty, Scott Owens, Susmit Sarkar, Peter Sewell, and Tjark Weber, “Mathematizing C++ Concurrency”, POPL 2011. * Damian Dechev, Peter Pirkelbauer, and Bjarne Stroustrup: Understanding and Effectively Preventing the ABA Problem in Descriptor-based Lock-free Designs. 13th IEEE Computer Society ISORC 2010 Symposium. May 2010. * Damian Dechev and Bjarne Stroustrup: Scalable Non-blocking Concurrent Objects for Mission Critical Code. ACM OOPSLA'09. October 2009 -* Damian Dechev, Peter Pirkelbauer, Nicolas Rouquette, and Bjarne Stroustrup: Semantically Enhanced Containers for Concurrent Real-Time Systems. Proc. 16th Annual IEEE International Conference and Workshop on the Engineering of Computer Based Systems (IEEE ECBS). April 2009. +* Damian Dechev, Peter Pirkelbauer, Nicolas Rouquette, and Bjarne Stroustrup: Semantically Enhanced Containers for Concurrent Real-Time Systems. Proc. 16th Annual IEEE International Conference and Workshop on the Engineering of Computer Based Systems (IEEE ECBS). April 2009. ### CP.110: Use a conventional pattern for double-checked locking @@ -11331,7 +11335,7 @@ Double-checked locking is easy to mess up. x_init.store(true, memory_order_release); } } - + // ... use x ... @@ -11356,7 +11360,7 @@ These rules defy simple catagorization: ##### Example const volatile long clock; - + This describes a register constantly updated by a clock circuit. `clock` is `volatile` because its value will change without any action from the C++ program that uses it. For example, reading `clock` twice will often yield two different values, so the optimizer had better not optimize away the second read in this code: @@ -11364,7 +11368,7 @@ For example, reading `clock` twice will often yield two different values, so the long t1 = clock; // ... no use of clock here ... long t2 = clock; - + `clock` is `const` because the program should not try to write to `clock`. ###### Note