ShowTime icon indicating copy to clipboard operation
ShowTime copied to clipboard

"Reference to class property 'enabled' is not concurrency-safe because it involves shared mutable state" with Strict Concurrency Checking = Complete

Open cderito opened this issue 2 years ago • 6 comments

Hello,

I tried enabling Complete Strict Concurrency Checking in Xcode 14.1 (in the Build Settings of my target) and a warning appeared on the line where I was setting ShowTime.enabled.

In particular my code is:

AppDelegate.swift

func applicationDidBecomeActive(_ application: UIApplication) {
    ShowTime.enabled = ...
    ...
}

Please find a minimal sample project attached, it's entirely SwiftUI so no AppDelegate but the behavior is the same. ShowTimeTest.zip

Thank you.

cderito avatar Nov 11 '22 11:11 cderito

Hey! Thank you for raising this and for putting together a sample project. I'll take a look and try and fix it :)

KaneCheshire avatar Nov 12 '22 12:11 KaneCheshire

Hey :) sorry for the delay. So I think the solution to this is to just annotate the ShowTime type with @MainActor to enforce that it and any of its properties etc can only be used in a @MainActor isolated environment.

This would mean that any place it's being used needs to be annotated with @MainActor as well; luckily UIApplicationDelegate already is annotated with @MainActor but if you're using any ShowTime property in a SwiftUI view you'll probably need to annotate the view with @MainActor explicitly, which is a bit annoying but is technically correct for async code.

KaneCheshire avatar Jan 17 '23 20:01 KaneCheshire

Hi @KaneCheshire, thanks for looking into it! By annotating the ShowTime type with @MainActor what do you mean exactly? Because in my code I'm directly using its static variables so I can't do something like

@MainActor let showTime = ShowTime()

or similar, and both the App and UIApplicationDelegate in which I use it are already annotated with @MainActor.

Do you mean I should create an extension for ShowTime and annotate it?

Thank you!

cderito avatar Jan 18 '23 13:01 cderito

@cderito oh sorry no I explained that badly. What I mean is, the ShowTime type itself needs to be updated in this repo to be annotated with @MainActor for strict concurrency. So it's nothing you're doing wrong at all, the compiler is just expecting us to isolate the getters and setters of the properties for strict concurrency.

My main concern with doing this is that even people who don't have strict concurrency mode enabled yet will be forced to add @MainActor to their SwiftUI views or anywhere they're accessing ShowTime properties in places not yet isolated to @MainActor 🤔

KaneCheshire avatar Jan 18 '23 20:01 KaneCheshire

Oh I see! I had a 1:1 with an Apple engineer about Swift Concurrency some time after opening this issue and I got to know that there's a @preconcurrency attribute that can be used inline with import declarations to silence warnings but I couldn't get it to silence this particular warning, it seems it's dedicated to Sendable related warnings.

Anyway this will probably become an error only once Swift 6 is out so I think the fix doesn't need to be immediate, it could just be planned in a roadmap to Swift 6, or rolled out at least once this flag becomes on by default in some Xcode release, what do you think about it?

cderito avatar Jan 19 '23 08:01 cderito

@KaneCheshire Any plans on updating the lib with a fix for this? Thanks for your effort on this lib.

mkoorn avatar Apr 08 '24 12:04 mkoorn