Skip to content

Commit c5a8dee

Browse files
author
lhh
committed
Add BRPC_SPAN_ENABLE_SHARED_PTR_API flag for backward-compatible Span lifecycle management (#3068)
1 parent d470862 commit c5a8dee

File tree

10 files changed

+109
-7
lines changed

10 files changed

+109
-7
lines changed

src/brpc/channel.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,11 @@ void Channel::CallMethod(const google::protobuf::MethodDescriptor* method,
510510
span->set_base_cid(correlation_id);
511511
span->set_protocol(_options.protocol);
512512
span->set_start_send_us(start_send_us);
513+
#if BRPC_SPAN_ENABLE_SHARED_PTR_API
513514
accessor.set_span(span);
515+
#else
516+
accessor.set_span(span.get());
517+
#endif
514518
}
515519
}
516520
// Override some options if they haven't been set by Controller

src/brpc/controller.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1727,4 +1727,31 @@ void Controller::DoPrintLogPrefix(std::ostream& os) const {
17271727
}
17281728
}
17291729

1730+
1731+
#if BRPC_SPAN_ENABLE_SHARED_PTR_API
1732+
ControllerPrivateAccessor& ControllerPrivateAccessor::set_span(
1733+
std::shared_ptr<Span> span) {
1734+
_cntl->_span = span;
1735+
return *this;
1736+
}
1737+
#else
1738+
ControllerPrivateAccessor& ControllerPrivateAccessor::set_span(Span* span) {
1739+
if (span) {
1740+
_cntl->_span = span->shared_from_this();
1741+
} else {
1742+
_cntl->_span.reset();
1743+
}
1744+
return *this;
1745+
}
1746+
#endif
1747+
1748+
SpanPtr ControllerPrivateAccessor::span() const {
1749+
std::shared_ptr<Span> span_internal = _cntl->_span.lock();
1750+
#if BRPC_SPAN_ENABLE_SHARED_PTR_API
1751+
return span_internal;
1752+
#else
1753+
return span_internal.get();
1754+
#endif
1755+
}
1756+
17301757
} // namespace brpc

src/brpc/details/controller_private_accessor.h

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,20 @@ class Message;
3030
}
3131
}
3232

33-
3433
namespace brpc {
3534

35+
class Span;
36+
37+
#ifndef BRPC_SPAN_ENABLE_SHARED_PTR_API
38+
#define BRPC_SPAN_ENABLE_SHARED_PTR_API 0
39+
#endif
40+
41+
#if BRPC_SPAN_ENABLE_SHARED_PTR_API
42+
using SpanPtr = std::shared_ptr<Span>;
43+
#else
44+
using SpanPtr = Span*;
45+
#endif
46+
3647
class AuthContext;
3748

3849
// A wrapper to access some private methods/fields of `Controller'
@@ -90,17 +101,18 @@ class ControllerPrivateAccessor {
90101
return *this;
91102
}
92103

93-
ControllerPrivateAccessor &set_span(std::shared_ptr<Span> span) {
94-
_cntl->_span = span;
95-
return *this;
96-
}
104+
#if BRPC_SPAN_ENABLE_SHARED_PTR_API
105+
ControllerPrivateAccessor &set_span(std::shared_ptr<Span> span);
106+
#else
107+
ControllerPrivateAccessor &set_span(Span* span);
108+
#endif
97109

98110
ControllerPrivateAccessor &set_request_protocol(ProtocolType protocol) {
99111
_cntl->_request_protocol = protocol;
100112
return *this;
101113
}
102114

103-
std::shared_ptr<Span> span() const { return _cntl->_span.lock(); }
115+
SpanPtr span() const;
104116

105117
uint32_t pipelined_count() const { return _cntl->_pipelined_count; }
106118
void set_pipelined_count(uint32_t count) { _cntl->_pipelined_count = count; }

src/brpc/policy/baidu_rpc_protocol.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,11 @@ void ProcessRpcRequest(InputMessageBase* msg_base) {
647647
span = Span::CreateServerSpan(
648648
request_meta.trace_id(), request_meta.span_id(),
649649
request_meta.parent_span_id(), msg->base_real_us());
650+
#if BRPC_SPAN_ENABLE_SHARED_PTR_API
650651
accessor.set_span(span);
652+
#else
653+
accessor.set_span(span.get());
654+
#endif
651655
span->set_log_id(request_meta.log_id());
652656
span->set_remote_side(cntl->remote_side());
653657
span->set_protocol(PROTOCOL_BAIDU_STD);

src/brpc/policy/http_rpc_protocol.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1506,7 +1506,11 @@ void ProcessHttpRequest(InputMessageBase *msg) {
15061506
}
15071507
span = Span::CreateServerSpan(
15081508
path, trace_id, span_id, parent_span_id, msg->base_real_us());
1509+
#if BRPC_SPAN_ENABLE_SHARED_PTR_API
15091510
accessor.set_span(span);
1511+
#else
1512+
accessor.set_span(span.get());
1513+
#endif
15101514
span->set_log_id(cntl->log_id());
15111515
span->set_remote_side(user_addr);
15121516
span->set_received_us(msg->received_us());

src/brpc/policy/hulu_pbrpc_protocol.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,11 @@ void ProcessHuluRequest(InputMessageBase* msg_base) {
416416
span = Span::CreateServerSpan(
417417
meta.trace_id(), meta.span_id(), meta.parent_span_id(),
418418
msg->base_real_us());
419+
#if BRPC_SPAN_ENABLE_SHARED_PTR_API
419420
accessor.set_span(span);
421+
#else
422+
accessor.set_span(span.get());
423+
#endif
420424
span->set_log_id(meta.log_id());
421425
span->set_remote_side(cntl->remote_side());
422426
span->set_protocol(PROTOCOL_HULU_PBRPC);

src/brpc/policy/nshead_protocol.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,11 @@ void ProcessNsheadRequest(InputMessageBase* msg_base) {
300300
std::shared_ptr<Span> span;
301301
if (IsTraceable(false)) {
302302
span = Span::CreateServerSpan(0, 0, 0, msg->base_real_us());
303+
#if BRPC_SPAN_ENABLE_SHARED_PTR_API
303304
accessor.set_span(span);
305+
#else
306+
accessor.set_span(span.get());
307+
#endif
304308
span->set_log_id(req_head->log_id);
305309
span->set_remote_side(cntl->remote_side());
306310
span->set_protocol(PROTOCOL_NSHEAD);

src/brpc/policy/sofa_pbrpc_protocol.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,11 @@ void ProcessSofaRequest(InputMessageBase* msg_base) {
376376
span = Span::CreateServerSpan(
377377
0/*meta.trace_id()*/, 0/*meta.span_id()*/,
378378
0/*meta.parent_span_id()*/, msg->base_real_us());
379+
#if BRPC_SPAN_ENABLE_SHARED_PTR_API
379380
accessor.set_span(span);
381+
#else
382+
accessor.set_span(span.get());
383+
#endif
380384
span->set_remote_side(cntl->remote_side());
381385
span->set_protocol(PROTOCOL_SOFA_PBRPC);
382386
span->set_received_us(msg->received_us());

src/brpc/policy/thrift_protocol.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,11 @@ void ProcessThriftRequest(InputMessageBase* msg_base) {
518518
std::shared_ptr<Span> span;
519519
if (IsTraceable(false)) {
520520
span = Span::CreateServerSpan(0, 0, 0, msg->base_real_us());
521+
#if BRPC_SPAN_ENABLE_SHARED_PTR_API
521522
accessor.set_span(span);
523+
#else
524+
accessor.set_span(span.get());
525+
#endif
522526
span->set_log_id(seq_id);
523527
span->set_remote_side(cntl->remote_side());
524528
span->set_protocol(PROTOCOL_THRIFT);

src/brpc/span.h

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,43 @@ extern __thread bthread::LocalStorage tls_bls;
4242

4343
namespace brpc {
4444

45+
// ============================================================================
46+
// Span Lifecycle Management API Compatibility Layer
47+
// ============================================================================
48+
//
49+
// COMPATIBILITY NOTE:
50+
// brpc uses std::shared_ptr<Span> internally
51+
// to prevent use-after-free bugs in async RPC callbacks.
52+
//
53+
// For backward compatibility with existing protocol extensions, external APIs
54+
// can return raw pointers (Span*) by default. To enable the modern shared_ptr
55+
// API, compile with -DBRPC_SPAN_ENABLE_SHARED_PTR_API=1.
56+
//
57+
// MIGRATION GUIDE:
58+
// - Legacy mode (default): SpanPtr = Span*
59+
// Users must ensure the Span outlives their usage (typically by keeping
60+
// the Controller alive).
61+
//
62+
// - Modern mode (recommended): SpanPtr = std::shared_ptr<Span>
63+
// Automatic lifetime management, safer for async operations.
64+
//
65+
// ============================================================================
66+
67+
#ifndef BRPC_SPAN_ENABLE_SHARED_PTR_API
68+
#define BRPC_SPAN_ENABLE_SHARED_PTR_API 0 // Default: legacy mode for compatibility
69+
#endif
70+
4571
class Span;
4672

73+
#if BRPC_SPAN_ENABLE_SHARED_PTR_API
74+
// Modern API: Return shared_ptr for safe lifecycle management
75+
using SpanPtr = std::shared_ptr<Span>;
76+
#else
77+
// Legacy API: Return raw pointer for backward compatibility
78+
// WARNING: Users must ensure the Span outlives their usage
79+
using SpanPtr = Span*;
80+
#endif
81+
4782
void SetTlsParentSpan(std::shared_ptr<Span> span);
4883
std::shared_ptr<Span> GetTlsParentSpan();
4984
void ClearTlsParentSpan();
@@ -252,7 +287,7 @@ friend class SpanContainer;
252287

253288
class SpanContainer : public bvar::Collected {
254289
public:
255-
explicit SpanContainer(std::shared_ptr<Span> span) : _span(span) {}
290+
explicit SpanContainer(const std::shared_ptr<Span>& span) : _span(span) {}
256291
~SpanContainer() {}
257292

258293
// Implementations of bvar::Collected

0 commit comments

Comments
 (0)