[#1549] Address Swift 6 concurrency warning by synchronizing access to verbosityLevel.

[jaredsinclair]

What and why?

When building a host application target that imports the Datadog iOS SDK and has complete concurrency checking enabled on the application target, Datadog.verbosityLevel emits the following warning:

Static property 'verbosityLevel' is not concurrency-safe because it is non-isolated global shared mutable state; this is an error in Swift 6

This PR addresses the defect using an existing synchronization utility within DatadogInternal.

How?

By downgrading Datadog.verbosityLevel to a computed property, backed by a private property using an instance of ReadWriteStorage<Value> to ensure efficient synchronization, the concurrency checker warning is fully resolved.

The warning that the concurrency checker emits, according to the setup described above, is a valid one. It's an example of the tooling doing exactly what it was designed to address, among other types of violations: shared mutable state not protected via explicit synchronization or actor isolation. Since this property has been and must continue to be readable and writable from any thread, and without regard to actor isolation, the most appropriate change is to wrap access to the property in a synchronization mechanism.

The ReadWriteLock class is the right tool for the job. It was missing only one key ingredient: an explicit declaration that it conforms to Sendable. It cannot directly conform to Sendable because it contains a private var property, mutable state. But the maintainers of the Datadog SDK know that it was designed to be Sendable before Sendable was a term, through the use of a POSIX reader/writer lock. Therefore it can be reasonably declared @unchecked Sendable, satisfying the concurrency checker through years of peer review and production exercise.

Review checklist

• Feature or bugfix MUST have appropriate tests (unit, integration) Existing tests that exercise verbosityLevel should suffice.
• Make sure each commit and the PR mention the Issue number or JIRA reference Setting the log verbosity level produces a strict concurrency warning #1549
• Add CHANGELOG entry for user facing changes

Custom CI job configuration (optional)

• Run unit tests for Core, RUM, Trace, Logs, CR and WVT
• Run unit tests for Session Replay
• Run integration tests
• Run smoke tests
• Run tests for tools/

[ganeshnj]

Suggested change

	public static var verbosityLevel: CoreLoggerLevel? {
	get { _verbosityLevel.wrappedValue }
	set { _verbosityLevel.wrappedValue = newValue }
	}
	/// The backing storage for `verbosityLevel`, ensuring efficient synchronized
	/// read/write access to the shared value.
	private static let _verbosityLevel = ReadWriteLock<CoreLoggerLevel?>(wrappedValue: nil)
	@ReadWriteLock
	public static var verbosityLevel: CoreLoggerLevel? = nil

I wonder if you really need a backing property here.

[jaredsinclair]

Unfortunately, property wrappers are unusable here; the Swift 6 / complete concurrency checker sees this as mutable state:

These two requirements are at odds:

• Property wrappers must be defined as var not let.
• The concurrency checker disallows the use of var because it's shared mutable state

Something has to give. Until the day that Swift gains the ability to overlook a var wrapped in a sendable property wrapper, it is not possible to use a property wrapper to resolve concurrency issues with shared mutable state.

We have run up against this at my workplace's projects as we've spent the last year working towards complete concurrency checking. We have tried many permutations of property wrapping (f.ex. experiments with nonmutating set) to no avail. The only alternative at this time (for use cases like verbosityLevel) is to use a backing property which can be defined as a let, satisfying the concurrency checker.

[jaredsinclair]

@ganeshnj Thanks. I've fixed the issue with my commit (it was unsigned) which required a force-push, which has wiped out the previous Bitrise run. I don't see a means of rerunning that check on my end.

[ganeshnj]

@jaredsinclair I just kicked off the build.

[jaredsinclair]

@ganeshnj What does this mean? It looks like all the steps passed except...whatever this top one represents:

[ganeshnj]

This is just false positive, I verified, everything looks fine. I think bitrise can't handle the PR created from forks correctly. This PR is good to go and we will take care of it. Someone from the team will merge and release in the next release.