chenBright commented on code in PR #3140:
URL: https://github.com/apache/brpc/pull/3140#discussion_r2936385819
##########
src/brpc/span.cpp:
##########
@@ -37,9 +37,96 @@
#define BRPC_SPAN_INFO_SEP "\1"
-
namespace brpc {
+// Callback for creating a new bthread span when creating a new bthread.
+// This is called by bthread layer when BTHREAD_INHERIT_SPAN flag is set.
+// Returns a heap-allocated weak_ptr<Span>* as void*, or NULL if span creation
fails.
+void* CreateBthreadSpanAsVoid() {
+ const int64_t received_us = butil::cpuwide_time_us();
+ const int64_t base_realtime = butil::gettimeofday_us() - received_us;
+ std::shared_ptr<Span> span = Span::CreateBthreadSpan("Bthread",
base_realtime);
+
+ if (!span) {
+ return NULL;
+ }
+ return new std::weak_ptr<Span>(span);
+}
+
+void DestroyRpczParentSpan(void* ptr) {
+ if (ptr) {
+ delete static_cast<std::weak_ptr<Span>*>(ptr);
+ }
+}
+
+void EndBthreadSpan() {
+ std::shared_ptr<Span> span = GetTlsParentSpan();
+ if (span) {
+ bthread_id_t id = {bthread_self()};
+ span->set_ending_cid(id);
+ }
+
+ ClearTlsParentSpan();
+}
+
+void SetTlsParentSpan(std::shared_ptr<Span> span) {
+ using namespace bthread;
+ LocalStorage ls = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_bls);
+ if (ls.rpcz_parent_span) {
+ *static_cast<std::weak_ptr<Span>*>(ls.rpcz_parent_span) = span;
+ } else {
+ ls.rpcz_parent_span = new std::weak_ptr<Span>(span);
+ BAIDU_SET_VOLATILE_THREAD_LOCAL(tls_bls, ls);
+ }
+}
+
+std::shared_ptr<Span> GetTlsParentSpan() {
+ using namespace bthread;
+ LocalStorage ls = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_bls);
+ if (!ls.rpcz_parent_span) {
+ return nullptr;
+ }
+
+ auto* weak_ptr = static_cast<std::weak_ptr<Span>*>(ls.rpcz_parent_span);
+ return weak_ptr->lock();
+}
+
+void ClearTlsParentSpan() {
+ using namespace bthread;
+ LocalStorage ls = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_bls);
+ if (ls.rpcz_parent_span) {
+ static_cast<std::weak_ptr<Span>*>(ls.rpcz_parent_span)->reset();
+ }
+}
+
+bool HasTlsParentSpan() {
+ using namespace bthread;
+ LocalStorage ls = BAIDU_GET_VOLATILE_THREAD_LOCAL(tls_bls);
+ if (!ls.rpcz_parent_span) {
+ return false;
+ }
+
+ auto* weak_ptr = static_cast<std::weak_ptr<Span>*>(ls.rpcz_parent_span);
+ return !weak_ptr->expired();
+}
+
+
+void SpanDeleter::operator()(Span* r) const {
+ if (r == NULL) {
+ return;
+ }
+
+ // All children will be destroyed automatically along with the list.
+ // The list holds std::shared_ptr<> which will trigger deletion of
+ // children.
+ r->_client_list.clear();
+ r->_info.clear();
+ // Destroy the spinlocks, as the destructor might not be invoked.
+ pthread_spin_destroy(&r->_client_list_spinlock);
+ pthread_spin_destroy(&r->_info_spinlock);
Review Comment:
It appears that the spinlock is only initialized once in the Span
constructor, but is destroyed once before each SpanDeleter::operator() call.
##########
src/brpc/span.cpp:
##########
@@ -37,9 +37,96 @@
#define BRPC_SPAN_INFO_SEP "\1"
-
namespace brpc {
+// Callback for creating a new bthread span when creating a new bthread.
+// This is called by bthread layer when BTHREAD_INHERIT_SPAN flag is set.
+// Returns a heap-allocated weak_ptr<Span>* as void*, or NULL if span creation
fails.
+void* CreateBthreadSpanAsVoid() {
+ const int64_t received_us = butil::cpuwide_time_us();
+ const int64_t base_realtime = butil::gettimeofday_us() - received_us;
+ std::shared_ptr<Span> span = Span::CreateBthreadSpan("Bthread",
base_realtime);
+
+ if (!span) {
+ return NULL;
+ }
+ return new std::weak_ptr<Span>(span);
+}
+
+void DestroyRpczParentSpan(void* ptr) {
+ if (ptr) {
+ delete static_cast<std::weak_ptr<Span>*>(ptr);
+ }
+}
+
+void EndBthreadSpan() {
+ std::shared_ptr<Span> span = GetTlsParentSpan();
+ if (span) {
+ bthread_id_t id = {bthread_self()};
Review Comment:
Why is bthread_t set to ending_cid?
##########
src/bthread/task_group.cpp:
##########
@@ -495,7 +507,11 @@ int TaskGroup::start_foreground(TaskGroup** pg,
m->attr = using_attr;
m->local_storage = LOCAL_STORAGE_INIT;
if (using_attr.flags & BTHREAD_INHERIT_SPAN) {
- m->local_storage.rpcz_parent_span = run_create_span_func();
+ if (g_create_bthread_span) {
+ m->local_storage.rpcz_parent_span = g_create_bthread_span();
Review Comment:
Who has a strong reference to bthread span?
Furthermore, after resolving the thread safety issues of spans, is it still
necessary to use bthread spans?
--
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]
