Repository navigation
Replace OSAllocatedUnfairLock with Mutex - #66
Merged
Merged
Conversation
Migrate locking from os.OSAllocatedUnfairLock to Mutex from the Synchronization framework. Where the lock guarded a single object, the Mutex now wraps that object directly: Baggage attributes, Meter's active instruments, the metric instruments' values, FlushTimer's suspended flag, the sampler GUID, and ExampleReporter's session GUID. Span and Tracer guarded several heterogeneous fields (some read elsewhere without locking), so they keep a single mutex that also serializes those fields, preserving the prior single-lock semantics. Read-only computed accessors were added where the wrapped value was previously a directly-readable stored property, to avoid churning the exporter and test call sites. Also adds public get accessors for `span` and `attributes` on Baggage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Span and Tracer guard several heterogeneous fields, so rather than having the Mutex wrap one of them as a stand-in, use a plain Mutex<Void> lock and keep the guarded fields as ordinary stored properties. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ladd
marked this pull request as ready for review
June 18, 2026 00:15
bachand
approved these changes
Jun 18, 2026
Comment on lines
+323
to
+325
| /// Serializes mutation of the guarded mutable state (`_attributes`, `events`, | ||
| /// `links`, `retireCallbacks`, `endTime`). | ||
| private let lock = Mutex<Void>(()) |
There was a problem hiding this comment.
What do you think about continuing to use OSAllocatedUnfairLock here and in other cases where the value is Void? I don't feel strongly, but the code did seem a bit cleaner without the _ ins. 🤔
Author
There was a problem hiding this comment.
I would ideally put the entire state storage inside the Mutex wrapper. I opted to save that for future refactor work.
ladd
force-pushed
the
lvt-baggage-visibility
branch
from
June 18, 2026 16:06
b33dc94 to
6322cb6
Compare
The test asserted exact handler-call counts against a free-running repeating timer, so a fire that landed in the window between an expectation completing and the next test step would break the equality checks. The shared counter was also mutated on the timer queue and read on the test thread. Guard the counter with a lock, drain any in-flight handler via a sync barrier on the (serial) timer queue after suspending, snapshot the count at suspend instead of asserting an exact value, and use an expectation (rather than == 2) to confirm the timer fires again after resume. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Guard the test's shared state with a Mutex from the Synchronization framework, consistent with the rest of the codebase. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates locking from
os.OSAllocatedUnfairLocktoMutexfrom theSynchronizationframework. This is a slight efficiency improvement (avoids an allocation), and wrapping the object is safer/clearer.Where the lock guarded a single object, the
Mutexnow wraps that object directly:Baggage→Mutex<TelemetryAttributes?>(also resolves the prior "stored property is mutable" Swift 6 warning)Meter→Mutex<[Instrument]>(active instruments)Counter/ObservableCounter/ObservableUpDownCounter/ObservableGauge/Histogram/ExponentialHistogram→Mutexwrapping the instrument'svaluesFlushTimer→Mutex<Bool>(suspended flag)StableGuidSampler→Mutex<Data>(GUID); thewithLockUncheckedset path became a normalwithLockExampleReporter→static Mutex<Data?>(session GUID)SpanandTracerguarded several heterogeneous fields (some read elsewhere without locking), so they keep a single mutex that also serializes those fields, preserving the prior single-lock semantics. Read-only computed accessors were added where the wrapped value was previously a directly-readable stored property, to avoid churning the exporter and test call sites.Also adds public get accessors for
spanandattributesonBaggage, for better ergonomics in client code.Test plan
swift build— clean (only the pre-existingBaggage.spannon-Sendable warning remains)swift test— all 92 tests pass.values.reset()were updated tosnapshotAndReset(), sincevaluesis now a read-only snapshot@jparise
@bachand