This is an automated email from the ASF dual-hosted git repository.
serverglen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new 491591cb Support release assert (#2306)
491591cb is described below
commit 491591cb15d8ad40a3e946df51ce36cdfe643036
Author: Bright Chen <[email protected]>
AuthorDate: Fri Jul 14 19:59:49 2023 +0800
Support release assert (#2306)
* Support release assert
* Support release assert
---
src/brpc/controller.cpp | 33 +++++++++++------------------
src/brpc/details/naming_service_thread.cpp | 4 ++--
src/brpc/http_method.cpp | 13 +++++-------
src/brpc/policy/dynpart_load_balancer.cpp | 10 +++------
src/bthread/countdown_event.cpp | 7 +++---
src/butil/containers/doubly_buffered_data.h | 8 +++----
src/butil/logging.h | 6 +++---
src/butil/macros.h | 30 ++++++++++++++++++++++++++
src/butil/memory/scoped_ptr.h | 3 +--
src/butil/scoped_generic.h | 5 +++--
src/bvar/detail/agent_group.h | 4 +---
src/bvar/mvariable.cpp | 7 +++---
src/bvar/variable.cpp | 14 ++++--------
test/brpc_channel_unittest.cpp | 6 ++----
14 files changed, 75 insertions(+), 75 deletions(-)
diff --git a/src/brpc/controller.cpp b/src/brpc/controller.cpp
index c9935a54..338ad66c 100644
--- a/src/brpc/controller.cpp
+++ b/src/brpc/controller.cpp
@@ -1508,26 +1508,20 @@ 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;
}
}
@@ -1535,13 +1529,10 @@ static void RegisterQuitSignalOrDie() {
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;
}
}
}
diff --git a/src/brpc/details/naming_service_thread.cpp
b/src/brpc/details/naming_service_thread.cpp
index 359ef38d..89648e77 100644
--- a/src/brpc/details/naming_service_thread.cpp
+++ b/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(
diff --git a/src/brpc/http_method.cpp b/src/brpc/http_method.cpp
index acb0de1c..b4f5fc43 100644
--- a/src/brpc/http_method.cpp
+++ b/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,9 +72,8 @@ 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),
@@ -83,10 +81,9 @@ static void BuildHttpMethodMaps() {
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);
diff --git a/src/brpc/policy/dynpart_load_balancer.cpp
b/src/brpc/policy/dynpart_load_balancer.cpp
index 4785a0f7..579ca7dd 100644
--- a/src/brpc/policy/dynpart_load_balancer.cpp
+++ b/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) {
diff --git a/src/bthread/countdown_event.cpp b/src/bthread/countdown_event.cpp
index f399a3c8..1c2c5952 100644
--- a/src/bthread/countdown_event.cpp
+++ b/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;
diff --git a/src/butil/containers/doubly_buffered_data.h
b/src/butil/containers/doubly_buffered_data.h
index 024a5ee8..e011d5da 100644
--- a/src/butil/containers/doubly_buffered_data.h
+++ b/src/butil/containers/doubly_buffered_data.h
@@ -331,9 +331,7 @@ private:
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);
diff --git a/src/butil/logging.h b/src/butil/logging.h
index 8138af34..c4f8ef00 100644
--- a/src/butil/logging.h
+++ b/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.
diff --git a/src/butil/macros.h b/src/butil/macros.h
index 6b7f4468..6519a105 100644
--- a/src/butil/macros.h
+++ b/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_
diff --git a/src/butil/memory/scoped_ptr.h b/src/butil/memory/scoped_ptr.h
index 61a44df4..0f435cd6 100644
--- a/src/butil/memory/scoped_ptr.h
+++ b/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()
diff --git a/src/butil/scoped_generic.h b/src/butil/scoped_generic.h
index 8310c713..d6d71971 100644
--- a/src/butil/scoped_generic.h
+++ b/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;
}
diff --git a/src/bvar/detail/agent_group.h b/src/bvar/detail/agent_group.h
index ceb7c05d..db327f27 100644
--- a/src/bvar/detail/agent_group.h
+++ b/src/bvar/detail/agent_group.h
@@ -172,9 +172,7 @@ private:
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;
}
diff --git a/src/bvar/mvariable.cpp b/src/bvar/mvariable.cpp
index 29caddff..bff17183 100644
--- a/src/bvar/mvariable.cpp
+++ b/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.
diff --git a/src/bvar/variable.cpp b/src/bvar/variable.cpp
index 9e623179..bd6de08a 100644
--- a/src/bvar/variable.cpp
+++ b/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.
diff --git a/test/brpc_channel_unittest.cpp b/test/brpc_channel_unittest.cpp
index f5806ccd..6d7d374b 100644
--- a/test/brpc_channel_unittest.cpp
+++ b/test/brpc_channel_unittest.cpp
@@ -76,10 +76,8 @@ public:
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;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
