swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.58k stars 10.36k forks source link

Swift 6.0.0.5 concurrency regression: incorrect data race diagnostic involving `lazy` #75106

Open NachoSoto opened 4 months ago

NachoSoto commented 4 months ago

Description

This used to compile with Xcode 16 beta 2.

Reproduction

final class T: Sendable {
  func f() async {}
}

@MainActor
final class S {
  func f() async {
    // Sending 'self.api' risks causing data races
    await api.f()
  }

  // Changing this to `private let` fixes the warning
  private lazy var api = h()
}

@MainActor
private func h() -> T { .init() }

Expected behavior

The code does not have any potential data races. "Sending self.api" should be safe since it's Sendable.

Environment

swift-driver version: 1.111.2 Apple Swift version 6.0 (swiftlang-6.0.0.5.15 clang-1600.0.22.6) Target: arm64-apple-macosx14.0

Additional information

Seems to be an incorrect diagnostic due to the use of lazy, which should be synchronized since S is @MainActor.

NachoSoto commented 4 months ago

cc @hborla

hborla commented 4 months ago

This appears to be fixed on main, @gottesmm any idea which change fixed it?

NachoSoto commented 3 months ago

This is still happening on Xcode 16 beta 4 (swift-driver version: 1.112.3 Apple Swift version 6.0 (swiftlang-6.0.0.6.8 clang-1600.0.23.1) (cc @hborla)

For some reason doesn't reproduce under swift repl or compiling through CLI, but it does when compiling through Xcode (both with Swift 5 and Swift 6 modes).

NachoSoto commented 3 months ago

This reproduces the issue:

swift -warnings-as-errors -warn-concurrency file.swift
/tmp/a.swift:13:15: error: sending 'self.api' risks causing data races; this is an error in the Swift 6 language mode
11 |   public func f() async {
12 |     // Sending 'self.api' risks causing data races
13 |     await api.f()
   |               |- error: sending 'self.api' risks causing data races; this is an error in the Swift 6 language mode
   |               `- note: sending main actor-isolated 'self.api' to nonisolated instance method 'f()' risks causing data races between nonisolated and main actor-isolated uses
14 |   }
15 |

Edit: also reproduces compiling with swift -swift-version 6 file.swift

NachoSoto commented 3 months ago

@gottesmm can you confirm this will be fixed on the next release?

NachoSoto commented 3 months ago

Looks like this isn't isolated to lazy. This reproduces it too:

final class T: Sendable {
  func f() async {}
}

@MainActor
final class S {
  func f() async {
    // Sending 'self.api' risks causing data races
    await api.f()
  }

  // Changing this to `private let` fixes the warning
  private var api: T { h() }
}

@MainActor
private func h() -> T { .init() }
NachoSoto commented 3 months ago

I should also say that adding isolated: isolated (any Actor)? = #isolation to T.f removes the error as well. But I don't understand why that's necessary when api is a var or lazy var but not when it's a let.

hborla commented 3 months ago

Your second example is also fixed on the release/6.0 branch.

can you confirm this will be fixed on the next release?

We cannot comment on future Xcode betas or releases. The best we can do here is check whether a given fix is included in a current Xcode beta. If you're interested in getting notified automatically when a fix lands in an Xcode beta, the best way to do that is to file a feedback report through Apple Feedback Assistant.

NachoSoto commented 3 months ago

Your second example is also fixed on the release/6.0 branch.

Great thank you so much!

For anyone else running into this, another workaround is to replace this

await api.f()

with this:

let api = self.api
await api.f()