Support release assert (#2306)

* Support release assert

* Support release assert

src/brpc/controller.cpp

@@ -1508,40 +1508,31 @@ static void RegisterQuitSignalOrDie() {
     SignalHandler prev = signal(SIGINT, quit_handler);
     if (prev != SIG_DFL &&
         prev != SIG_IGN) { // shell may install SIGINT of background jobs with SIG_IGN
-        if (prev == SIG_ERR) {
-            LOG(ERROR) << "Fail to register SIGINT, abort";
-            abort();
-        } else {
-            s_prev_sigint_handler = prev;
-            LOG(WARNING) << "SIGINT was installed with " << prev;
-        }
+        RELEASE_ASSERT_VERBOSE(prev != SIG_ERR,
+                               "Fail to register SIGINT, abort");
+        s_prev_sigint_handler = prev;
+        LOG(WARNING) << "SIGINT was installed with " << prev;
     }
 
     if (FLAGS_graceful_quit_on_sigterm) {
         prev = signal(SIGTERM, quit_handler);
         if (prev != SIG_DFL &&
             prev != SIG_IGN) { // shell may install SIGTERM of background jobs with SIG_IGN
-            if (prev == SIG_ERR) {
-                LOG(ERROR) << "Fail to register SIGTERM, abort";
-                abort();
-            } else {
-                s_prev_sigterm_handler = prev;
-                LOG(WARNING) << "SIGTERM was installed with " << prev;
-            }
+            RELEASE_ASSERT_VERBOSE(prev != SIG_ERR,
+                                   "Fail to register SIGTERM, abort");
+            s_prev_sigterm_handler = prev;
+            LOG(WARNING) << "SIGTERM was installed with " << prev;
         }
     }
 
     if (FLAGS_graceful_quit_on_sighup) {
         prev = signal(SIGHUP, quit_handler);
         if (prev != SIG_DFL &&
             prev != SIG_IGN) { // shell may install SIGHUP of background jobs with SIG_IGN
-            if (prev == SIG_ERR) {
-                LOG(ERROR) << "Fail to register SIGHUP, abort";
-                abort();
-            } else {
-                s_prev_sighup_handler = prev;
-                LOG(WARNING) << "SIGHUP was installed with " << prev;
-            }
+            RELEASE_ASSERT_VERBOSE(prev != SIG_ERR,
+                                   "Fail to register SIGHUP, abort");
+            s_prev_sighup_handler = prev;
+            LOG(WARNING) << "SIGHUP was installed with " << prev;
         }
     }
 }

src/brpc/details/naming_service_thread.cpp

@@ -80,13 +80,13 @@ NamingServiceThread::Actions::~Actions() {
 void NamingServiceThread::Actions::AddServers(
     const std::vector<ServerNode>&) {
     // FIXME(gejun)
-    abort();
+    RELEASE_ASSERT_VERBOSE(false, "Not implemented");
 }
 
 void NamingServiceThread::Actions::RemoveServers(
     const std::vector<ServerNode>&) {
     // FIXME(gejun)
-    abort();
+    RELEASE_ASSERT_VERBOSE(false, "Not implemented");
 }
 
 void NamingServiceThread::Actions::ResetServers(

src/brpc/http_method.cpp

@@ -16,7 +16,6 @@
 // under the License.
 
 
-#include <stdlib.h>                     // abort()
 #include "butil/macros.h"
 #include "butil/logging.h"
 #include <pthread.h>
@@ -73,20 +72,18 @@ struct LessThanByName {
 static void BuildHttpMethodMaps() {
     for (size_t i = 0; i < ARRAY_SIZE(g_method_pairs); ++i) {
         const int method = (int)g_method_pairs[i].method;
-        if (method < 0 || method > (int)ARRAY_SIZE(g_method2str_map)) {
-            abort();
-        }
+        RELEASE_ASSERT(method >= 0 &&
+                       method <= (int)ARRAY_SIZE(g_method2str_map));
         g_method2str_map[method] = g_method_pairs[i].str;
      }
     std::sort(g_method_pairs, g_method_pairs + ARRAY_SIZE(g_method_pairs),
               LessThanByName());
     char last_fc = '\0';
     for (size_t i = 0; i < ARRAY_SIZE(g_method_pairs); ++i) {
         char fc = g_method_pairs[i].str[0];
-        if (fc < 'A' || fc > 'Z') {
-            LOG(ERROR) << "Invalid method_name=" << g_method_pairs[i].str;
-            abort();
-        }
+        RELEASE_ASSERT_VERBOSE(fc >= 'A' && fc <= 'Z',
+                               "Invalid method_name=%s",
+                               g_method_pairs[i].str);
         if (fc != last_fc) {
             last_fc = fc;
             g_first_char_index[fc - 'A'] = (uint8_t)(i + 1);

src/brpc/policy/dynpart_load_balancer.cpp

@@ -127,13 +127,9 @@ int DynPartLoadBalancer::SelectServer(const SelectIn& in, SelectOut* out) {
                 && Socket::Address(id, &ptrs[nptr].first) == 0) {
                 int w = schan::GetSubChannelWeight(ptrs[nptr].first->user());
                 total_weight += w;
-                if (nptr < 8) {
-                    ptrs[nptr].second = total_weight;
-                    ++nptr;
-                } else {
-                    CHECK(false) << "Not supported yet";
-                    abort();
-                }
+                RELEASE_ASSERT_VERBOSE(nptr < 8, "Not supported yet");
+                ptrs[nptr].second = total_weight;
+                ++nptr;
             }
         }
         if (nptr != 0) {

src/bthread/countdown_event.cpp

@@ -26,10 +26,9 @@
 namespace bthread {
 
 CountdownEvent::CountdownEvent(int initial_count) {
-    if (initial_count < 0) {
-        LOG(FATAL) << "Invalid initial_count=" << initial_count;
-        abort();
-    }
+    RELEASE_ASSERT_VERBOSE(initial_count >= 0,
+                           "Invalid initial_count=%d",
+                           initial_count);
     _butex = butex_create_checked<int>();
     *_butex = initial_count;
     _wait_was_invoked = false;

src/butil/containers/doubly_buffered_data.h

@@ -331,9 +331,7 @@ class DoublyBufferedData<T, TLS, AllowBthreadSuspended>::WrapperTLSGroup {
     inline static std::deque<WrapperTLSId>& _get_free_ids() {
         if (BAIDU_UNLIKELY(!_s_free_ids)) {
             _s_free_ids = new (std::nothrow) std::deque<WrapperTLSId>();
-            if (!_s_free_ids) {
-                abort();
-            }
+            RELEASE_ASSERT(_s_free_ids);
         }
         return *_s_free_ids;
     }
@@ -510,8 +508,8 @@ template <typename T, typename TLS, bool AllowBthreadSuspended>
 DoublyBufferedData<T, TLS, AllowBthreadSuspended>::DoublyBufferedData()
     : _index(0)
     , _wrapper_key(0) {
-    static_assert(!(AllowBthreadSuspended && !IsVoid<TLS>::value),
-                  "Forbidden to allow suspend bthread with non-Void TLS");
+    BAIDU_CASSERT(!(AllowBthreadSuspended && !IsVoid<TLS>::value),
+                  "Forbidden to allow bthread suspended with non-Void TLS");
 
     _wrappers.reserve(64);
     pthread_mutex_init(&_modify_mutex, NULL);

src/butil/logging.h

@@ -489,19 +489,19 @@ void print_vlog_sites(VLogSitePrinter*);
 
 // file/line can be specified at running-time. This is useful for printing
 // logs with known file/line inside a LogSink or LogMessageHandler
-#define GET_LOG_AT_MACRO(_1, _2, _3, _4, NAME, ...) NAME
+#define LOG_AT_SELECTOR(_1, _2, _3, _4, NAME, ...) NAME
 
 #define LOG_AT_STREAM1(severity, file, line)                                 \
     ::logging::LogMessage(file, line, ::logging::BLOG_##severity).stream()
 #define LOG_AT_STREAM2(severity, file, line, func)                           \
     ::logging::LogMessage(file, line, func, ::logging::BLOG_##severity).stream()
-#define LOG_AT_STREAM(...) GET_LOG_AT_MACRO(__VA_ARGS__, LOG_AT_STREAM2, LOG_AT_STREAM1)(__VA_ARGS__)
+#define LOG_AT_STREAM(...) LOG_AT_SELECTOR(__VA_ARGS__, LOG_AT_STREAM2, LOG_AT_STREAM1)(__VA_ARGS__)
 
 #define LOG_AT1(severity, file, line)                                        \
     BAIDU_LAZY_STREAM(LOG_AT_STREAM(severity, file, line), LOG_IS_ON(severity))
 #define LOG_AT2(severity, file, line, func)                                   \
     BAIDU_LAZY_STREAM(LOG_AT_STREAM(severity, file, line, func), LOG_IS_ON(severity))
-#define LOG_AT(...) GET_LOG_AT_MACRO(__VA_ARGS__, LOG_AT2, LOG_AT1)(__VA_ARGS__)
+#define LOG_AT(...) LOG_AT_SELECTOR(__VA_ARGS__, LOG_AT2, LOG_AT1)(__VA_ARGS__)
 
 
 // The VLOG macros log with negative verbosities.

src/butil/macros.h

@@ -12,8 +12,10 @@
 
 #include <stddef.h>  // For size_t.
 #include <string.h>  // For memcpy.
+#include <stdlib.h>
 
 #include "butil/compiler_specific.h"  // For ALLOW_UNUSED.
+#include "butil/string_printf.h"      // For butil::string_printf().
 
 // There must be many copy-paste versions of these macros which are same
 // things, undefine them to avoid conflict.
@@ -442,4 +444,32 @@ namespace {  /*anonymous namespace */                           \
 
 #endif  // __cplusplus
 
+#define ASSERT_LOG(fmt, ...)                                            \
+    do {                                                                \
+        std::string log = butil::string_printf(fmt, ## __VA_ARGS__);    \
+        LOG(FATAL) << log;                                              \
+    } while (false)
+
+// Assert macro that can crash the process to generate a dump.
+#define RELEASE_ASSERT(condition)   \
+    do {                            \
+        if (!(condition)) {         \
+            ::abort();              \
+        }                           \
+    } while (false)
+
+// Assert macro that can crash the process to generate a dump and
+// supply a verbose explanation of what went wrong.
+// For example:
+//  std::vector<int> v;
+//  ...
+//  RELEASE_ASSERT_VERBOSE(v.empty(), "v should be empty, but with size=%zu", v.size());
+#define RELEASE_ASSERT_VERBOSE(condition, fmt, ...)                                 \
+    do {                                                                            \
+        if (!(condition)) {                                                         \
+            ASSERT_LOG("Assert failure: " #condition ". " #fmt, ## __VA_ARGS__);    \
+            ::abort();                                                              \
+        }                                                                           \
+    } while (false)
+
 #endif  // BUTIL_MACROS_H_

src/butil/memory/scoped_ptr.h

@@ -223,8 +223,7 @@ class scoped_ptr_impl {
 
   void reset(T* p) {
     // This is a self-reset, which is no longer allowed: http://crbug.com/162971
-    if (p != NULL && p == data_.ptr)
-      abort();
+    RELEASE_ASSERT(p == NULL || p != data_.ptr);
 
     // Note that running data_.ptr = p can lead to undefined behavior if
     // get_deleter()(get()) deletes this. In order to prevent this, reset()

src/butil/scoped_generic.h

@@ -11,6 +11,7 @@
 
 #include "butil/compiler_specific.h"
 #include "butil/move.h"
+#include "butil/macros.h"
 
 namespace butil {
 
@@ -96,8 +97,8 @@ class ScopedGeneric {
   // object, if given. Self-resets are not allowd as on scoped_ptr. See
   // http://crbug.com/162971
   void reset(const element_type& value = traits_type::InvalidValue()) {
-    if (data_.generic != traits_type::InvalidValue() && data_.generic == value)
-      abort();
+    RELEASE_ASSERT(data_.generic == traits_type::InvalidValue() ||
+                   data_.generic != value);
     FreeIfNecessary();
     data_.generic = value;
   }

src/bvar/detail/agent_group.h

@@ -172,9 +172,7 @@ class AgentGroup {
     inline static std::deque<AgentId> &_get_free_ids() {
         if (__builtin_expect(!_s_free_ids, 0)) {
             _s_free_ids = new (std::nothrow) std::deque<AgentId>();
-            if (!_s_free_ids) {
-                abort();
-            }
+            RELEASE_ASSERT(_s_free_ids);
         }
         return *_s_free_ids;
     }

src/bvar/mvariable.cpp

@@ -167,10 +167,9 @@ int MVariable::expose_impl(const butil::StringPiece& prefix,
         }
     }
 
-    if (FLAGS_bvar_abort_on_same_name) {
-        LOG(FATAL) << "Abort due to name conflict";
-        abort();
-    } else if (!s_bvar_may_abort) {
+    RELEASE_ASSERT_VERBOSE(!FLAGS_bvar_abort_on_same_name,
+                           "Abort due to name conflict");
+    if (!s_bvar_may_abort) {
         // Mark name conflict occurs, If this conflict happens before
         // initialization of bvar_abort_on_same_name, the validator will
         // abort the program if needed.

src/bvar/variable.cpp

@@ -48,12 +48,7 @@ DEFINE_bool(bvar_abort_on_same_name, false,
 // Remember abort request before bvar_abort_on_same_name is initialized.
 bool s_bvar_may_abort = false;
 static bool validate_bvar_abort_on_same_name(const char*, bool v) {
-    if (v && s_bvar_may_abort) {
-        // Name conflict happens before handling args of main(), this is
-        // generally caused by global bvar.
-        LOG(FATAL) << "Abort due to name conflict";
-        abort();
-    }
+    RELEASE_ASSERT_VERBOSE(!v || !s_bvar_may_abort, "Abort due to name conflict");
     return true;
 }
 const bool ALLOW_UNUSED dummy_bvar_abort_on_same_name = ::GFLAGS_NS::RegisterFlagValidator(
@@ -167,10 +162,9 @@ int Variable::expose_impl(const butil::StringPiece& prefix,
             return 0;
         }
     }
-    if (FLAGS_bvar_abort_on_same_name) {
-        LOG(FATAL) << "Abort due to name conflict";
-        abort();
-    } else if (!s_bvar_may_abort) {
+    RELEASE_ASSERT_VERBOSE(!FLAGS_bvar_abort_on_same_name,
+                           "Abort due to name conflict");
+    if (!s_bvar_may_abort) {
         // Mark name conflict occurs, If this conflict happens before
         // initialization of bvar_abort_on_same_name, the validator will
         // abort the program if needed.

test/brpc_channel_unittest.cpp

@@ -76,10 +76,8 @@ class DeleteOnlyOnceChannel : public brpc::Channel {
     DeleteOnlyOnceChannel() : _c(1) {
     }
     ~DeleteOnlyOnceChannel() {
-        if (_c.fetch_sub(1) != 1) {
-            LOG(ERROR) << "Delete more than once!";
-            abort();
-        }
+        RELEASE_ASSERT_VERBOSE(_c.fetch_sub(1) == 1,
+                               "Delete more than once!");
     }
 private:
     butil::atomic<int> _c;