Make segment/subsegment closing thread-safe
- Dominant language
- Java
- Stars
- 100
- Forks
- 100
- PR merge metrics
- No merged PRs in 30d
Description
When closing a subsegment, we check if its parent segment is completed and has no other subsegment. If so we will emit the segment as well. This is because when we close a segment which parents live subsegments, we don't emit it but wait for all child subsegments complete. However, the current implementation is not thread-safe per below:
Customer ask:
"
Looking at the AWS X-Ray Java SDK code:
* https://github.com/aws/aws-xray-sdk-java/blob/master/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java#L496
* https://github.com/aws/aws-xray-sdk-java/blob/master/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java#L53
The problems here are that (a) `LongAdder` is something of an "eventually consistent" counter and (b) checking `isInProgress` and `referenceCount` doesn't happen atomically. The end result is that both the segment and the subsegment itself may attempt to emit the subsegment.
To fix this, the following changes are needed:
* `referenceCount` must be turned into an `AtomicInteger` instead of `LongAdder`.
* The reference count must include the segment itself, i.e. it must start at 1 and reach zero when the segment has been closed. With this change, the separate `!isInProgress()` check can and must be removed.
* `Decrementing` and checking the value must be done atomically, i.e. `AtomicInteger#decrementAndGet()` instead of `LongAdder#decrement() followed by LongAdded#intValue()`.
"
Contributor guide
Assessment
This issue has not been assessed yet.