From 335876a1335c765f818ae10d9c4d18f563cdfce5 Mon Sep 17 00:00:00 2001 From: Victor Costan Date: Wed, 22 Dec 2021 19:12:56 +0000 Subject: [PATCH] Add invariant checks to Limiter in Env implementations. PiperOrigin-RevId: 417853172 --- util/env_posix.cc | 33 ++++++++++++++++++++++++++++++--- util/env_windows.cc | 24 ++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/util/env_posix.cc b/util/env_posix.cc index 9ac03f8..8b8d9c8 100644 --- a/util/env_posix.cc +++ b/util/env_posix.cc @@ -73,7 +73,14 @@ Status PosixError(const std::string& context, int error_number) { class Limiter { public: // Limit maximum number of resources to |max_acquires|. - Limiter(int max_acquires) : acquires_allowed_(max_acquires) {} + Limiter(int max_acquires) + : +#if !defined(NDEBUG) + max_acquires_(max_acquires), +#endif // !defined(NDEBUG) + acquires_allowed_(max_acquires) { + assert(max_acquires >= 0); + } Limiter(const Limiter&) = delete; Limiter operator=(const Limiter&) = delete; @@ -86,15 +93,35 @@ class Limiter { if (old_acquires_allowed > 0) return true; - acquires_allowed_.fetch_add(1, std::memory_order_relaxed); + int pre_increment_acquires_allowed = + acquires_allowed_.fetch_add(1, std::memory_order_relaxed); + + // Silence compiler warnings about unused arguments when NDEBUG is defined. + (void)pre_increment_acquires_allowed; + // If the check below fails, Release() was called more times than acquire. + assert(pre_increment_acquires_allowed < max_acquires_); + return false; } // Release a resource acquired by a previous call to Acquire() that returned // true. - void Release() { acquires_allowed_.fetch_add(1, std::memory_order_relaxed); } + void Release() { + int old_acquires_allowed = + acquires_allowed_.fetch_add(1, std::memory_order_relaxed); + + // Silence compiler warnings about unused arguments when NDEBUG is defined. + (void)old_acquires_allowed; + // If the check below fails, Release() was called more times than acquire. + assert(old_acquires_allowed < max_acquires_); + } private: +#if !defined(NDEBUG) + // Catches an excessive number of Release() calls. + const int max_acquires_; +#endif // !defined(NDEBUG) + // The number of available resources. // // This is a counter and is not tied to the invariants of any other class, so diff --git a/util/env_windows.cc b/util/env_windows.cc index 84905df..9ffcd07 100644 --- a/util/env_windows.cc +++ b/util/env_windows.cc @@ -114,7 +114,14 @@ class ScopedHandle { class Limiter { public: // Limit maximum number of resources to |max_acquires|. - Limiter(int max_acquires) : acquires_allowed_(max_acquires) {} + Limiter(int max_acquires) + : +#if !defined(NDEBUG) + max_acquires_(max_acquires), +#endif // !defined(NDEBUG) + acquires_allowed_(max_acquires) { + assert(max_acquires >= 0); + } Limiter(const Limiter&) = delete; Limiter operator=(const Limiter&) = delete; @@ -133,9 +140,22 @@ class Limiter { // Release a resource acquired by a previous call to Acquire() that returned // true. - void Release() { acquires_allowed_.fetch_add(1, std::memory_order_relaxed); } + void Release() { + int old_acquires_allowed = + acquires_allowed_.fetch_add(1, std::memory_order_relaxed); + + // Silence compiler warnings about unused arguments when NDEBUG is defined. + (void)old_acquires_allowed; + // If the check below fails, Release() was called more times than acquire. + assert(old_acquires_allowed < max_acquires_); + } private: +#if !defined(NDEBUG) + // Catches an excessive number of Release() calls. + const int max_acquires_; +#endif // !defined(NDEBUG) + // The number of available resources. // // This is a counter and is not tied to the invariants of any other class, so