apache
Span::CreateClientSpan是否有线程安全问题
Describe the bug (描述bug)
https://github.com/apache/brpc/blob/master/src/brpc/span.cpp#L133
在代码这个位置，如果是在多个bthread中调用的Span::CreateClientSpan，而他们属于同一个Span的child span。此时他们对brpc::Span::_next_client是存在竞态的。 即便认为这个指针的赋值是原子的，似乎也可能导致内存泄漏。
这里是有什么特殊的设计避免了竞态吗？求解答
To Reproduce (复现方法)
Expected behavior (期望行为)
Versions (各种版本) OS: Compiler: brpc: protobuf:
Additional context/screenshots (更多上下文/截图)
补充一下，我跑的测试代码
brpc部分，就是随便加点输出信息。
如果同时写parent->_next_client，则可能写坏内存，导致出core。这个就比较难复现了。
上述问题可以通过使用cas进行链表插入来解决，当然brpc的span不止这一处问题，收到请求包后发起异步调用，svr span会被析构。收到异步调用回包后再发起rpc就可能core。done->Run()提前回包后再发起rpc也有类似的问题。 为了解决这个问题我们对svr span加入了引用计数机制。在合适的位置为当前的svr span引用计数+1，析构时-1。算是比较彻底的解决了这个问题。 在这些问题解决前，不建议在生产环境使用brpc的trace。
上述问题可以通过使用cas进行链表插入来解决，当然brpc的span不止这一处问题，收到请求包后发起异步调用，svr span会被析构。收到异步调用回包后再发起rpc就可能core。done->Run()提前回包后再发起rpc也有类似的问题。 为了解决这个问题我们对svr span加入了引用计数机制。在合适的位置为当前的svr span引用计数+1，析构时-1。算是比较彻底的解决了这个问题。 在这些问题解决前，不建议在生产环境使用brpc的trace。
hello，想请教一下“收到异步调用回包后再发起rpc就可能core”，异步回调中发起rpc时，所在的bthread的thread local的变量中，应该不包含原来那个svr span的指针吧。因为这可能会是一个新的bthread，那么会是什么原因导致的core呢？
从源代码来看（https://github.com/apache/brpc/blob/master/src/brpc/controller.cpp#L718），创建新的bthread的时候，也没有使用BTHREAD_INHERIT_SPAN标志。brpc这个时候，可能不认为新的rpc请求是老的svr span的child span了。不知道我理解的是否有问题 @bigolcy
我们的代码与主干相距过远了，具体原因可能得看看现在的代码实现是否还有问题。https://github.com/apache/brpc/blob/master/src/brpc/span.cpp#L422 没记错的话，这里析构会把client span一起析构。异步发包后再访问client span就可能core了，跟是否有rpc应该关系不大。
上述问题可以通过使用cas进行链表插入来解决，当然brpc的span不止这一处问题，收到请求包后发起异步调用，svr span会被析构。收到异步调用回包后再发起rpc就可能core。done->Run()提前回包后再发起rpc也有类似的问题。 为了解决这个问题我们对svr span加入了引用计数机制。在合适的位置为当前的svr span引用计数+1，析构时-1。算是比较彻底的解决了这个问题。 在这些问题解决前，不建议在生产环境使用brpc的trace。
hello，想请教一下“收到异步调用回包后再发起rpc就可能core”，异步回调中发起rpc时，所在的bthread的thread local的变量中，应该不包含原来那个svr span的指针吧。因为这可能会是一个新的bthread，那么会是什么原因导致的core呢？
从源代码来看（https://github.com/apache/brpc/blob/master/src/brpc/controller.cpp#L718），创建新的bthread的时候，也没有使用BTHREAD_INHERIT_SPAN标志。brpc这个时候，可能不认为新的rpc请求是老的svr span的child span了。不知道我理解的是否有问题 @bigolcy
感觉你可以在父bthread里面创建多个span，然后作为argument传递给每个子bthread。
@wwbmmm 这个问题我最近想出一个方案来解决，想和你讨论一下： 现在启动bthread如果加上BTHREAD_INHERIT_SPAN标志，bthread会继承span，为了修复这个问题，简单的方法是在创建bthread的时候，如果有BTHREAD_INHERIT_SPAN这个标志，就执行一次Span::CreateClientSpan，创建一个client span，让这个client span成为m->local_storage.rpcz_parent_span。 这样做的 优势在于，创建span仍然是能够完全规避了线程竞争，所以对性能应该很友好。 缺点在于，会增加一些span的数量。 实现难度很低，但是也会有个问题，就是需要在bthread层面调用Span::CreateClientSpan，但是Span::CreateClientSpan是在brpc这个层面上，代码调用关系可能有些不合理。
优势在于，创建span仍然是能够完全规避了线程竞争，所以对性能应该很友好。
这个修改没有想象中那么简单，至少还涉及了rpcz相关的代码。这里会出现多个trace id，span id一样的span对象。 那么存入leveldb时，可能要考虑是否合并到一起。 如果不合并则要考虑rpcz的展示需要做一些修改。
所以，加一个锁，或者一个lockfree的队列会来的更快。
另外还注意到TRACEPRINTF这个宏其实可能也有线程安全问题，最终其实是调用到Span::Annotate这个函数。 如果是多个线程同时写，则显然需要一个锁。
所以总的来说，除非trace部分整体重新设计一遍，否则，使用锁是一个看上去更清晰的方案。
优势在于，创建span仍然是能够完全规避了线程竞争，所以对性能应该很友好。
这个修改没有想象中那么简单，至少还涉及了rpcz相关的代码。这里会出现多个trace id，span id一样的span对象。 那么存入leveldb时，可能要考虑是否合并到一起。 如果不合并则要考虑rpcz的展示需要做一些修改。
所以，加一个锁，或者一个lockfree的队列会来的更快。
另外还注意到TRACEPRINTF这个宏其实可能也有线程安全问题，最终其实是调用到Span::Annotate这个函数。 如果是多个线程同时写，则显然需要一个锁。
所以总的来说，除非trace部分整体重新设计一遍，否则，使用锁是一个看上去更清晰的方案。
单线程调用内部多次调用createClientSpan，这个应该不会造成重复spanId吧，多线程调用Annotate相当于多线程并发处理一个请求？
优势在于，创建span仍然是能够完全规避了线程竞争，所以对性能应该很友好。
这个修改没有想象中那么简单，至少还涉及了rpcz相关的代码。这里会出现多个trace id，span id一样的span对象。 那么存入leveldb时，可能要考虑是否合并到一起。 如果不合并则要考虑rpcz的展示需要做一些修改。 所以，加一个锁，或者一个lockfree的队列会来的更快。 另外还注意到TRACEPRINTF这个宏其实可能也有线程安全问题，最终其实是调用到Span::Annotate这个函数。 如果是多个线程同时写，则显然需要一个锁。 所以总的来说，除非trace部分整体重新设计一遍，否则，使用锁是一个看上去更清晰的方案。
单线程调用内部多次调用createClientSpan，这个应该不会造成重复spanId吧，多线程调用Annotate相当于多线程并发处理一个请求？
如果span id不相同，那么是不是会意味着逻辑上的一个span，在代码中实际上被处理成多个不同的span了？当然，处理成多个不同的span倒是也还蛮好，或许可以参考open telemetry，使用一个新的span类型，比如叫internal span。
关于TRACEPRINTF，我想表达的意思是，在现有的情况下，是对bthread::tls_bls.rpcz_parent_span这个对象进行写入，当多个线程同时写入的时候，会不会有线程安全问题？不过按照你描述的这种改法来看，修改后应该不存在这个问题了。 https://github.com/apache/brpc/blob/master/src/brpc/span.cpp#L298
实现难度很低，但是也会有个问题，就是需要在bthread层面调用Span::CreateClientSpan，但是Span::CreateClientSpan是在brpc这个层面上，代码调用关系可能有些不合理。
这个可以让bthread开放一个set_create_span_func的方法，然后在brpc代码里将真正实现的方法设置进去
实现难度很低，但是也会有个问题，就是需要在bthread层面调用Span::CreateClientSpan，但是Span::CreateClientSpan是在brpc这个层面上，代码调用关系可能有些不合理。
这个可以让bthread开放一个set_create_span_func的方法，然后在brpc代码里将真正实现的方法设置进去
#2519