aws / aws/aws-xray-sdk-java

Make segment/subsegment closing thread-safe

Open
#108 8 comments 2 reactions 0 assignees View on GitHub
bug
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.