chenBright commented on code in PR #3647:
URL: https://github.com/apache/brpc/pull/3647#discussion_r4358014016
##########
src/brpc/span.cpp:
##########
@@ -506,10 +506,15 @@ static int64_t g_last_delete_tm = 0;
// Following variables are monitored by builtin services, thus non-static.
static pthread_mutex_t g_span_db_mutex = PTHREAD_MUTEX_INITIALIZER;
+// Serializes persistence with shutdown.
+static pthread_mutex_t g_span_dump_mutex = PTHREAD_MUTEX_INITIALIZER;
static bool g_span_ending = false; // don't open span again if this var is
true.
// Can't use intrusive_ptr which has ctor/dtor issues.
static SpanDB* g_span_db = nullptr;
-bool has_span_db() { return !!g_span_db; }
+bool has_span_db() {
+ BAIDU_SCOPED_LOCK(g_span_db_mutex);
+ return g_span_db != nullptr;
+}
Review Comment:
There is no recursive locking path in the current implementation.
`g_span_db_mutex` is private to `span.cpp`, and the existing callers of
`has_span_db()` do not hold it. No code inside a DB-lock critical section calls
this function.
The lock fixes the previously unsynchronized read of `g_span_db`. An
unlocked helper is unnecessary for the current call sites, so I am keeping this
implementation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
