Add lock to SpanContext to protect baggage #83
Conversation
Signed-off-by: Isaac Hier <isaachier@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #83 +/- ##
==========================================
+ Coverage 88.52% 88.55% +0.03%
==========================================
Files 96 96
Lines 2291 2298 +7
==========================================
+ Hits 2028 2035 +7
Misses 263 263
Continue to review full report at Codecov.
|
|
@rnburn could you please take a look at this and see if it looks OK? @yurishkuro this is the change for C++ client to lock baggage in span context to avoid thread-unsafe return-by-reference semantics in OpenTracing C++ (opentracing/opentracing-cpp#74). |
|
@black-adder or @vprithvi do you mind glancing over this? The background isn't so important just I had to make SpanContext protect baggage with a mutex. |
| @@ -160,9 +166,17 @@ class SpanContext : public opentracing::SpanContext { | |||
|
|
|||
| bool operator==(const SpanContext& rhs) const | |||
| { | |||
| { | |||
| std::lock(_mutex, rhs._mutex); | |||
| std::lock_guard<std::mutex> lock(_mutex, std::adopt_lock); | |||
There was a problem hiding this comment.
can this deadlock? this grabs its own lock and rhs grabs its own lock and then we're stuck?
There was a problem hiding this comment.
I was hoping you wouldn't pay too much attention to this. std::lock is a cool trick in C++11 that locks two mutexes without causing deadlock (essentially a loop of try_lock under the hood):
Locks the given Lockable objects lock1, lock2, ..., lockn using a deadlock avoidance algorithm to avoid deadlock.
http://en.cppreference.com/w/cpp/thread/lock
The lock guard stuff is just to make sure we unlock, doesn't acquire the lock again. Serves same purpose as defer mutex.Unlock() would in Go.
|
Thanks @black-adder |
Signed-off-by: Isaac Hier isaachier@gmail.com