SE-0412: Strict concurrency for global variables

Hello, Swift community!

The review of SE-0412: Strict concurrency for global variables begins now and runs through November 22nd, 2023.

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to me as the review manager by email or DM. When contacting the review manager directly, please put "SE-0412" in the subject line.

Trying it out

If you'd like to try this proposal out, you can download a toolchain supporting it for Linux , for Windows, or for macOS. You will need to add -enable-experimental-feature GlobalConcurrency to your build flags.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to Swift?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at:

https://github.com/apple/swift-evolution/blob/main/process.md

Thank you,

Holly Borla
Review Manager

14 Likes

:+1:

Seems pretty straightforward.

The only thing I'm a bit unsure about is the language interoperability aspect. It seems like it might be too easy for someone - very familiar with Swift and used to the (v6+) compiler ensuring their code is concurrency-safe - to innocuously import some C++ library and mistakenly believe their use of it is similarly protected by the compiler. I realise that the C++ code might be inherently unsafe, or at least is capable of doing plenty of unsafe things with its globals, but it'd be nice if we didn't so casually permit that lack of safety to cross over into the Swift code. At least with respect to writing Swift code that then extends that lack of safety [unwittingly].

I'm not sure what the solution might be - I guess options might include requiring explicit thread safety annotations for the C++ imports (even if that's just nonisolated(unsafe))… although without infrastructure to actually integrate into actors from the C++ side, I'm not sure that's effective. Or perhaps the users of such imported globals should have to write something extra (in the same vein as try) to better ensure they realise they're doing something unsafe.

2 Likes

Seems pretty straightforward! My only question is whether we can add any runtime assertions to catch data races in the case of manual synchronization. These checks could be enabled with nonisolated(unchecked). Of course, users would still have the option of skipping these assertions for performance reasons using the proposed nonisolated(unsafe). Other than that the proposal looks good.

+1

I've been eagerly awaiting this for a long time. I think it's a vital part of concurrency checking.

And it's not because I use many global variables (or that I think they're particularly common in Swift code); it's because eliminating data races is all about accesses to shared mutable state, and I can only think of a handful of categories of SMS that you really find in Swift programs:

  1. Global variables
  2. Classes
  3. C/C++/Obj-C stuff
  4. Actors (safe)
  5. Unsafe pointers (explicitly unsafe)

So I think it's a big deal to be able to cross global variables off that list (or at least, mark them as safe). Even if you don't use them a lot, somebody else might, and it's nice to have the assurance that their authors have had to think about isolation. It's important to close these gaps.

6 Likes

Yes certainly runtime checking is essential for manually managed synchronization. Such dynamic analysis is already available from exclusivity enforcement (and mentioned in the "detailed design" section accordingly).

1 Like

I certainly hear this concern. I don't see a great solution for it though, which isn't to say that we can't keep thinking about it and taking feedback. Like requiring explicit annotation of nonisolated(unsafe) rather than implicitly synthesizing it, while requiring some attention, won't actually make such imported globals thread-safe, and for imported library code that can't be functionally changed or fundamentally can't be made thread-safe by specification (like most standard C library globals), the annotation will be required for interop but will not provide any safety, only language compatibility.

The idea of extra syntax is interesting, but I can't yet think of a version that would enforce anything.

The best approaches that I can think of so far are what are described in the proposal, which is to enable exclusivity enforcement to have a better chance of catching violations and create your own thread-safe wrappers for access to unsafe C or C++ globals. Perhaps the latter could be better automated or shortcut, but I think that could be addressed in a future proposal.

1 Like

Looks like this plays nicely with global atomics, locks, and concurrent data structures because those would all be "immutable" (declared with let) as well as being Sendable when the things they are holding are also Sendable. :+1: I assume such globals are not implicitly isolated to @MainActor?

1 Like

Would it be an option to rename unsafe global variables when importing from C/C++? For example

// C header
int fooGlobal;

// Swift
@preconcurrency(unsafe) import CModule // or some other annotation 

unsafeFooGlobal += 1

The unsafe prefix would only be added when the import is annotated. If an unsafe global is imported without the import statement being annotated, the compiler could emit a warning. By only renaming the variable when the import is annotated, this should be source compatible.

This would make it harder to accidentally use an unsafe global variable, just because it’s imported.

1 Like

That's a good question, and https://github.com/apple/swift-evolution/blob/main/proposals/0343-top-level-concurrency.md actually specifies that top-level global variables are always implicitly @MainActor isolated, though I realize that would not be desirable in instances like you describe. This is only the case for script-mode code though, so I think it is sufficient to have the workaround in that case be to place global declarations of such concurrency primitive types within a struct as static to achieve get around the implicit main actor isolation.

My concern is that that would be massively non-obvious and therefore confusing when a developer encounters some imported symbols as available as declared and others not because some of them were surreptitiously renamed, with no visibility to the developer that that happened.

4 Likes

This is a great proposal: short, too-the-point, and critical for concurrency safety. All of the pieces in here are things I've expected since we introduced the concurrency model, and I have little to say about them. Just two comments:

There may be need in some circumstances to opt out of static checking to enable the developer to rely upon their own data isolation management, such as with an associated global lock serializing data access. The attribute nonisolated(unsafe) can be used to annotate the global variable (or any form of storage). Though this will disable static checking of data isolation for the global variable

Aside from the "any form of storage" parenthetical, this is talking exclusively about global variables, but I think it applies equally to local variables that have been captured. For example, I've often seen patterns where a local variable is captured in a @Sendable closure that's executed exactly once, like this:

func myFunc() {
  var myValue = 17
  doSomethingOnAnotherThread {
    myValue = 19
  }
  print(myValue)
}

This is going to produce an error, because myValue is being accessed from a Sendable closure. Can I use nonisolated(unsafe) on myValue to suppress the diagnostic when I know this is safe?

Imported C or C++ global variables, unless they are immutable and Sendable , are treated as if they have unsafely opted out of isolation checking, depending upon the developer to know how to use them safely.

Instead of talking about imported variables, can this be described in terms of @preconcurrency? For example, we could say that it's okay to access a global variable that fails all of the criteria (it's not on a global actor, is mutable, or is non-Sendable) if that global variable is in a module imported with a preconcurrency import. C/C++ global variables are imported as @preconcurrency, so this formulation subsumes the one in the proposal while being more based on "Swift semantics" rather than treating C/C++ declarations as special.

Doug

6 Likes

Does this require an explicit @preconcurrency import for C/C++ modules? I’m wondering if accessing an unsafe C/C++ global variable could generate a warning without the annotation being explicitly added?

(Taking off my review manager hat)

All imported clang declarations are @preconcurrency without use of a @preconcurrency import statement. The existing behavior from the implementation of SE-0337 is whether or not an error related to use of a @preconcurrency declaration is downgraded to a warning or suppressed depends on the -strict-concurrency checking level (e.g. "minimal checking" or "complete checking"). Diagnostics are suppressed with minimal checking, and downgraded to warnings with complete checking. A @preconcurrency import statement always silences diagnostics relating to declarations from the imported library* (and you get a warning at the site of a @preconcurrency import if no diagnostics are suppressed because of it).

*the SE-0337 proposal states that the diagnostics from @preconcurrency imported declarations will be warnings in Swift 6, though they are suppressed in Swift 5 mode with -strict-concurrency=complete.

The exact rule for import-as-nonisolated(unsafe) formulated in terms of @preconcurrency depends on the detailed design of the rule.

2 Likes

Can I use nonisolated(unsafe) on myValue to suppress the diagnostic when I know this is safe?

@Douglas_Gregor now you can: [Sema] nonisolated(unsafe) for local variables by sophiapoirier · Pull Request #70135 · apple/swift · GitHub :slight_smile:

Instead of talking about imported variables, can this be described in terms of @preconcurrency?

Great suggestion and I am working on the revision.

Wonderful, thank you!

Doug

+1 Nice to see a directive like nonisolated(unsafe) to opt out of the Sendability checking when needed.

Thanks everyone for your review discussion so far! The review has been extended until December 12th. Please continue the review discussion in this thread.

Holly Borla
Review Manager

4 Likes

The revised proposal looks great to me, thank you to the authors!

Doug

2 Likes