Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting the log verbosity level produces a strict concurrency warning #1549

Closed
jpsim opened this issue Nov 9, 2023 · 8 comments
Closed

Setting the log verbosity level produces a strict concurrency warning #1549

jpsim opened this issue Nov 9, 2023 · 8 comments
Assignees

Comments

@jpsim
Copy link

jpsim commented Nov 9, 2023

Setting the log verbosity level produces a strict concurrency warning when compiling with SWIFT_STRICT_CONCURRENCY = complete or -strict-concurrency=complete produces the following warning:

Datadog.verbosityLevel = .debug
// 👆 Reference to static property 'verbosityLevel' is not concurrency-safe because it involves shared mutable state

Ideally Datadog's iOS SDKs would be audited for strict concurrency warnings so that consumers can enable these strict flags if they choose to do so without introducing warnings.

@jpsim
Copy link
Author

jpsim commented Nov 9, 2023

I can file a separate issue for this if you prefer, but it would also be nice if LoggerProtocol was Sendable.

@ncreated ncreated self-assigned this Nov 14, 2023
@ncreated
Copy link
Collaborator

Hello @jpsim 👋. This request seems to overlap with #1440.

Ideally Datadog's iOS SDKs would be audited for strict concurrency warnings so that consumers can enable these strict flags if they choose to do so without introducing warnings.

This is something we already track in our backlog. I will bump up the priority with this new ask.

@farshadtx
Copy link

Any update here?

@ncreated
Copy link
Collaborator

ncreated commented Mar 6, 2024

@farshadtx Unfortunately no, this topic is still in our backlog.

@jaredsinclair
Copy link
Contributor

@ncreated Howdy, I would like to know, when you write ...is still in our backlog... did that mean it's being actively worked on? Do ya'll have a target date by which you plan to be ready for Swift 6 and strict concurrency?

jaredsinclair added a commit to jaredsinclair/dd-sdk-ios that referenced this issue Apr 15, 2024
Explicitly declares Sendable conformance for ReadWriteLock.

Uses ReadWriteLock to Synchronize access to Datadog.verbosityLevel.
@jaredsinclair
Copy link
Contributor

I've opened a PR from my fork of the repo that addresses the defect using an existing synchronization within the SDK:

#1778

jaredsinclair added a commit to jaredsinclair/dd-sdk-ios that referenced this issue Apr 19, 2024
Explicitly declares Sendable conformance for ReadWriteLock.

Uses ReadWriteLock to Synchronize access to Datadog.verbosityLevel.
jaredsinclair added a commit to jaredsinclair/dd-sdk-ios that referenced this issue Apr 19, 2024
Explicitly declares Sendable conformance for ReadWriteLock.

Uses ReadWriteLock to Synchronize access to Datadog.verbosityLevel.
ncreated added a commit that referenced this issue Apr 23, 2024
…urrency-swift-6-error

[#1549] Address Swift 6 concurrency warning by synchronizing access to verbosityLevel.
@ncreated
Copy link
Collaborator

Thanks a lot @jaredsinclair 👍. I just merged the PR, so it will be available in the next release (likely 2.11.0).

ncreated added a commit that referenced this issue Apr 26, 2024
* develop: (43 commits)
  Update apiSurface files for Swift and Objective-C
  Update dependency-manager-tests/spm/Shared/DatadogSetup.swift
  Bumped version to 2.10.0
  Update CHANGELOG for 2.10.0 release
  Revert nil interface
  Revert "Lock identifier assertion to default device"
  Lock identifier assertion to default device
  Tests for store update
  Fix lint
  Update store version
  Fix color SR identifier
  PR fixes
  Synchronize access to Datadog.verbosityLevel fixing issue #1549
  RUM-3134 attach vc to window
  Fix deployment target on SPM project
  Apply suggestions from code review
  apply cr suggestions
  Documentation update
  PR fixes
  Remove failing test
  ...
@ncreated
Copy link
Collaborator

2.11.0 is out and it includes the fix. Thanks everyone for participating!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants