Capture of self... in a '@Sendable'... Please make it stop!

Xcode 16.2 RC is giving new warnings

The queue is a DispatchQueue

I don't want to fix anything (the code isn't broken) - I just want to make this warning go away.

What's the minimal solution for that?

I am assured that swift 6 concurrency checking is opt-in (@hborla ), so it should be simple right?

1 Like

It would be useful to know a bit more about self (aka AnalysisLoader) here. A few possibilities:

  • If self doesn't have any mutable internal state, you may be able to simply add a AnalysisLoader: Sendable conformance.

  • If self has mutable internal state, but uses a queue to synchronize access to it (maybe HSImageRequeseQueue?), you can annotate AnalysisLoader with @unchecked Sendable.

  • If self has mutable internal state and doesn't use actors, queues nor locks to synchronize access to it, but analyse(image: CGImage, actualSeconds: TimeInterval) -> FrameAnalysis doesn't touch self at all, maybe you can declare it static and avoid capturing self completely, without needing AnalysisLoader to be sendable.

  • ...

In any case, since you're accessing AnalysisLoader across different threads and the code isn't broken, I take it something else is ensuring that access across multiple threads is safe. I believe @unchecked Sendable is precisely the right tool for when a type has been made safe to be used across multiple threads through a mechanism the compiler can't reason about (like locks, queues...).

4 Likes

It has no mutable state, but I can't mark it sendable as it has an internal variable

private let models: [String: VNCoreMLModel]

If I mark as sendable, I get

Stored property 'models' of 'Sendable'-conforming class 'AnalysisLoader' has non-sendable type '[String : VNCoreMLModel]'; this is an error in the Swift 6 language mode

Who has time for this?

how do I make it just go away?

So - now I just have to mark everything @unchecked Sendable to use concurrency?

Do I now risk deliberately compiler-induced crashes if I break some unwritten rule?

https://jaredsinclair.com/2024/11/12/beware-unchecked.html

Breaking sendable conformance rules won't necessarily lead to crashes, but it could lead to hard-to-diagnose data races which cause unexpected behavior.

@unchecked Sendable is the officially recommended way to say "I'm sychronizing this with a lock/queue/etc that the compiler doesn't know how to verify."

1 Like

You keep trying to fight with the compiler, but it has no bad intentions. The warnings you get is simply (and from its perspective — validly) here to catch your attention: there might be an issue with concurrency, so you can pause for a moment and analyse.

In your case here there are clearly potential accesses from concurrent contexts in unsafe way — from compiler perspective. models I suppose has reference types stored there, and can be eventually modified from different isolation, introducing concurrency issue. I keep repeating — it’s evaluation of a compiler with the given input and abilities to reason.

Now, if you are sure that you won’t access models in unsafe way or manage synchronization on your own, you can just mark type as @unchecked Sendable, saying to compiler “I’m sure this OK, I’ve got it, trust me”.

As for the crashes you are referring to, they are valid if you violated this promise to a compiler and actually access in unsafe way. And with concurrency issues, I’d say you better crash right away rather then have this thing live somewhere as bug corrupting data.

3 Likes

Surely you must have also seen this warning at the top of the file:

Add '@preconcurrency' to suppress 'Sendable'-related warnings from module 'Vision'

Doing so will let you conform AnalysisLoader to Sendable if indeed the only property in the object was the private let models: [String: VNCoreMLModel] (no need to use @unchecked Sendable).

Not at all. Lots of things are Sendable out of the box, for exmple. Swift will even add the Sendable conformance to your types when possible.

Plus, with minimal concurrency checking (which I assume is your setting), the code compiles just fine too.

Nope. This only happens when deliberately opting-in to the Swift 6 language mode.

2 Likes

I mean, that variable is exactly called mutable state. :thinking:

1 Like

yup - ok, that's technically true.

but in a way I really don't care about.

I mark the variable as a let - I load some models on init - I'm going to use them without changing them.

As far as I'm concerned in practice, there is no mutable state.

I realise that as far as the academic project that is swift 6 is concerned - there is a VNCoreMLModel that is technically mutable.

The fact that the compiler can't prove I'm not going to do dumb stuff is just an annoyance to me. I really wish it would just butt-out...

1 Like

Perhaps not - but it's deeply annoying!
The compiler should work for me - not the other way around.

I'm becoming increasingly annoyed by it's perspective.

Also - this is a giant hammer. In order to clear a warning in one place - I need to make a declaration about the whole class. And keep that in future.

Does @unchecked Sendable simply turn off warnings - or does it influence compilation in any way?

That's fine as long as I'm not making promises that I then need to keep...

The crashes in the example article were a deliberate compiler created crash. What promise exactly am I making that I need to keep to avoid the compiler deliberately crashing?

(what I'd prefer is if I could just keep in Swift 5 mode and not need to make promises to the compiler for it to stop bugging me)

Thanks for confirming this.

  1. That warning does not appear.

It appeared in a different file - perhaps the warning doesn't appear twice???

  1. Adding it doesn't make the warning go away

I mean, either your AnalysisLoader class is safe to use from multiple threads or it isn't. If it is safe (that's the only thing you're promising with @unchecked Sendable), then it'll always be Sendable. If it isn't, then Swift seems to be warning correctly here that you're using a non-thread-safe object across multiple threads.

The warning appears after adding Sendable to AnalysisLoader. The following compiles for me without warnings, even with strict concurrency enabled:

@preconcurrency import Vision

final class AnalysisLoader: Sendable {
    
    let models: [VNCoreMLModel]
    
    init(models: [VNCoreMLModel]) {
        self.models = models
    }
    
    func analyse() {}
    
    func analyse() async {
        DispatchQueue.global().async {
            self.analyse() // <-- No warning
        }
    }
}

That's highly depends on what you consider "works for you". From my perspective, catching concurrency using strict logic is working for me – it helps me to avoid bugs in code and be more sure on what I wrote.

Even though definition of this can be thought as a personal preference, I'd argue if freedom is beneficial in places that really easy to get wrong. We have C/C++/ObjC/etc languages that really give us freedom to work with pointers as we would like, and we – the same people who ask for this – introduce countless crashes by null pointer access. I mean, everybody did this mistake, everybody will make such mistake at any point in time in the future using these languages. And yet, in early Swift days there were arguing about Optional.

Sendable is a marker protocol, it doesn't exist in runtime.

Well, that's tough question. Technically, nobody makes you to keep this promise, yet if you happen to use the type you marked in such way from different concurrency domains, compiler won't spot it — you are the one who responsible for making sure this won't happen. And this would be a regular concurrency bug then.

The article shows different issue AFAICT, and honestly that seems like a bug to me – it is odd that closure kept this assertion. For @unchecked Sendable the only promise you make that your type is safe to use from different concurrency domains at once, so Swift will allow unsafely pass and mutate it however you want, and if something goes wrong with the data – it's on your watch only.

2 Likes

I do get what you mean, but mutable states is like what whole programming is about, should be really thought carefully.

It can't look into the future, though. VNCoreMLModel is a not thread safe class and could potentially be changed unnoticed, which would lead to different problems.

Tbh I'm not sure I understand code on initial screenshot, why DispatchQueue even needed?

1 Like

You’re dispatching some work to a queue but then you suspend the current task until that work completes. If we remove that, then all that remains is a recursive call to self.analyze() to forward the same arguments. So isn’t your function simply equivalent to this?

func analyze(image: CGImage, actualSeconds: TimeInterval) async {
  self.analyze(image: image, actualSeconds: actualSeconds)
}

The queue is a global one limiting concurrent Vision requests.
It stops Vision from getting overwhelmed and crashing.

If I was rewriting that now, I'd probably use an AsyncSemaphore

Is there another non-async overload of self.analyse() that gets called inside the closure though? As shown your code leads to infinite recursion.

Just to confirm, are you building with minimal concurrency checking? You can look for this either in your project build settings or in your build log. If you've opted into targeted or strict concurrency checking, then that means you've opted into concurrency checking. If you're building with minimal checking, this sounds like a compiler bug because the Sendable violation should be suppressed.

1 Like

FWIW, the below test case exhibits the following behavior:

  • -strict-concurrency=minimal: no diagnostics
  • -strict-concurrency=complete: warning
  • -swift-version 6: error
class C {
  func f() {}
}

func g(_: @Sendable () -> ()) {}

func h() {
  let c = C()
  g {
    c.f()
  }
}

I believe that sounds right?

3 Likes

i think we can deduce that there must be because the second function is being called in a synchronous context (a work item execution block).

1 Like