SE-0327 (Second Review): On Actors and Initialization

Evolution Proposal Reviews
1

Hello Swift community,

The second review of SE-0327 "On Actors and Initialization" begins now and runs through December 22, 2021.

This proposal has undergone a number of revisions in response to feedback from the first review, which the author has summarized as:

  1. Actor initializers that are not isolated to the actor will now allow you to do anything you normally can from a nonisolated method. In exchange, Swift will automatically reject accesses to stored properties that might be unsafe. Here is the problem description and proposed solution .
  2. Deinitializers of an actor can no longer access non-sendable stored properties of the instance. Here is the problem description and proposed solution
  3. The default value of a type's stored property is evaluated in a non-isolated context. Here is the problem description and proposed solution
  4. The convenience keyword is no longer required to define a delegating initializer of an actor. Here is the problem description and proposed rules for their delegating initializers, which is continued in the Sendability section.

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 the review manager. When emailing the review manager directly, please keep the proposal link at the top of the message.

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/master/process.md

Thank you,

Doug Gregor
Review Manager

11 Likes
2

It is hard to properly review this proposal. I don't have enough time to go to the proper depth of that this proposal needs / deserves. I mostly agree with the design choices made with a cursory reading but I can't tell if they will stand a closer scrutiny.

I just want to point out an issue that I think needs attention:

There is a serious teachability barrier for actor initializer rules because of the subtleties and complexities involved. I think this proposal needs to address teachability and progressive disclosure of these rules head-on.

Spending some time on figuring out an optimal teaching path and a good design for compiler diagnostics (to aid in the learning / discovery path) will shed more light on the validity of the design itself and will also make the proposal more reviewable.

6 Likes
3

In the vast majority of cases, the rules for actor initializers can be explained simply as:

  • Async actor initializers have actor isolation and work like any other initializer: a method that needs to initialize the stored properties before using self.
  • Non-async actor initializers do not have actor isolation and are like a nonisolated method that needs to initialize the stored properties before using self. But, if you use self for anything other than accessing its stored properties, from that point onwards within that initializer, you can't access those stored properties anymore.

All of the other unusual cases for an actor type, like a global-actor isolated initializer or a nonisolated async initializer, fall under the same category as non-async initializers.

So, while the proposal models this idea as "flow-sensitive isolation", that's just one way to describe it so that it fits into Swift's existing concepts for actors. The diagnostics in my in-progress implementation for these rules makes no mention of isolation changing at all. For example, the current diagnostics look like this:

actor Casey {
  var money: Money

  // some contrived examples of nonisolated methods
  nonisolated func speak(_ msg: String) { print("Casey: \(msg)") }
  nonisolated func cashUnderMattress() -> Int { return 0 }

  init() {
    money = Money(dollars: 100)
    defer { logTransaction(money.euros) }
//                         ^~~~~  error: cannot access stored property of 'self' here in non-isolated initializer
    self.speak("Yay, I have $\(money.dollars)!")
//  ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  note: after this use of 'self', its stored properties are no longer accessible in this initializer
  }

  init(with start: Int) {
    money = Money(dollars: start)

    if (money.dollars < 0) {
      self.speak("Oh no, I'm in debt!")
//    ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~  note: after this use of 'self', its stored properties are no longer accessible in this initializer
    }
    logTransaction(money.euros)
//                 ^~~~~  error: cannot access stored property of 'self' here in non-isolated initializer

    money.dollars += cashUnderMattress()
//  ^~~~~~~~~~~~~  error: cannot access stored property of 'self' here in non-isolated initializer
//                   ^~~~~~~~~~~~~~~~~~~  note: after this use of 'self', its stored properties are no longer accessible in this initializer
  }
}

Part of this proposal states that the diagnostics should always point-out a use of self to blame for causing a stored property access to be rejected. So, the example above shows three interesting situations for diagnostics, from top-to-bottom:

  1. A defer that accesses self.money is an error because it will be run after a call to self.speak.
  2. Reading self.money is an error after a conditional call to self.speak().
  3. A += mutation of self.money is an error because the update will happen after a call to self.cashUnderMattress(), which happens to be part of the same statement.

Those aren't the only reasons for emitting an error diagnostic, i.e., after the conditional call to self.speak(), the += update of self.money is an error too. But, in this version of the implementation, I have the compiler pointing-out closest use that causes the error diagnostic.

Keep in mind that for a non-async nonisolated method, you wouldn't be able to access any of these properties because it would require an await. So, the primary difference for a non-isolated initializer is that you start-off with the ability to access the stored properties, but lose that ability when it's no longer safe to do so.

1 Like
4

I spent a lot of time with actor initializers, and as well with Kavon syncing up about this work, so might as well provide my summary for review here.

  • Yes I think the isolation model proposed sounds good, and although it has some annoying pieces to it, it does achieve race safety and is work-able in general.
  • The proposal addresses a significant missing piece in our race-safety journey.
  • Spent many hours battling actor initializers and reading the various version of the proposal.

Short notes;

If the friend were Sendable , then executor access is not needed, because the two friend instances would not be mutable.

Nitpick; it's not about mutable or not; it could be externally synchronized.

Non-delegating initializers

I'm happy with how we ended up with phrasing everything in terms of delegating or not. Huge yay also for the removal of convenience :partying_face: Is

Implicit hop to self on fully-initialized

This works well and I'm in favor of this in general. It makes async initializers work as expected really. It is the non-async initializers which suffer limitations which is annoying but we're forced into this design somewhat.

nonisolated self initializers "decaying" the isolation

This took a while to internalize all the specific implications, but it makes sense and I'm in favor of this.

It means that we're allowed to make "one final escape of self" at the end of such initializer, which is important and useful. It's also equivalent to just adding the same line of code as "right after the init" basically.

I would really really want to get good diagnostics here. This model is sound and solid, but understanding what is going on from just "self is not isolated" messages that differ depending on line by line, can be quite confusing without proper explanation. Errors here should highlight the place where the self "decayed":

  init(_ initialScore: Int) {
    self.score = initialScore
    greetCharlie(self)  // note: self became nonisolated because of escaping use here
 
    assert(score >= 50) // ❌ error: cannot access mutable isolated storage after `nonisolated` use of `self`
}

Parameters of initializers must be sendable

This rule I struggled with getting to terms with for a while to be honest. It often is an actor's purpose to get passed in some non-sendable thing and it shall from there-onwards always manage it.

It is true however that our language does not make this safe. What we want to express in these scenarios is "move this not sendable thing to the actor", and therefore the actor shall only manage it from here onwards. With the upcoming introduction of move() and move only semantics, we'll be able to express what I alluded to here in the type system, rather than hand-waving away the issue. Until then we'll have to use unsafe wrappers to pass such things to actors, and while unfortunate it's an ok workaround I suppose...

Really looking forward to be able to:

// today:
  _ = await Gene(with: ns) // ❌ error: cannot pass non-Sendable value across actors
  _ = await Gene(with: move(ns)) // ✅

or however we'll decide to spell these in the future.

Related: Distributed actor initializers

The proposal works well with distributed actors. We discuss the interaction in-depth in the distributed actor runtime pitch's "readying distributed actors" section. Summing up though:

  • distributed actors have a concept of becoming ready for remote calls
  • in inits with nonisolated self this is done at the end of initializers
  • in inits with isolated self this is done immediately as the actor becomes fully-initialized, same as this proposal's "hop to self"
4 Likes
5

(Removing my review-manager hat)

+1. I think we've hit the sweetest spot we can with this proposal, with the simplest model that fits in the language and provides data race safety.

Yes, absolutely! Data-race safety is the core feature of actors, and this proposal addresses one of the most glaring issues with data race safety in today's language.

Yes!

I don't know of any others that have addresses this, specifically.

In-depth study; I've been involved in all of the pitch and review discussions.

Doug

2 Likes
6

merry christmas everyone! this proposal is really abstract and theoretical so i don’t understand it well enough to review it in full, and i won’t try. however, i have been using the recent toolchains extensively and i’ve run into some issues with initializing non-Sendable stored properties of an actor.

actor Greg 
{
  var ns:NonSendableType

  // Option 1: a non-delegating init can only take 
  // Sendable values and use them to construct 
  // a new non-Sendable value.
  init(fromPieces ps:(NonSendableType.Piece, NonSendableType.Piece)) 
  {
    self.ns = .init(ps)
  }

as i understand the current behavior, the non-isolated designated init runs in the same concurrency domain as its caller, and each = assignment to a stored property sends it over the boundary to the actor’s concurrency domain. this prevents the following from compiling

extension AsyncStream 
{
    static 
    func create(_ policy:Continuation.BufferingPolicy = .unbounded) 
        -> (consumer:Continuation, producer:Self)
    {
        var escapee:Continuation?       = nil 
        let producer:Self               = .init(bufferingPolicy: policy) 
        {
            escapee                     = $0
        }
        guard let consumer:Continuation = escapee 
        else 
        {   
            unreachable
        }
        return (consumer, producer) 
    }
}

actor Delegate 
{
    let control:AsyncStream<Signal>.Continuation
    private 
    let signals:AsyncStream<Signal>
    
    init()
    {
        // error, cannot send non-Sendable `$temporary.producer` 
        // to destination `self.signals`!
        (self.control, self.signals) = AsyncStream<Signal>.create()
    }
}

because this is treated as equivalent to

actor Delegate 
{
    let control:AsyncStream<Signal>.Continuation
    let signals:AsyncStream<Signal>
    // (imaginary synthesized init)
}

func delegate() -> Delegate
{
    let control:AsyncStream<Signal>.Continuation,
        signals:AsyncStream<Signal>
    (control, signals) = AsyncStream<Signal>.create()
    // would not, and should not compile:
    return Delegate.init(control: control, signals: signals) 
}

with SE-0327, does this mean that instead of a separate concurrency wall protecting each stored property from the body of the init, there will now be a single wall separating the body of the designated init from the caller context? will this now allow us to actually run initializers and static constructors for non-Sendable properties in an actor init? if so, this is hugely motivating, since at the moment, we cannot use actors with non-Sendable stored properties in practice, since such properties cannot be initialized in the first place.

7

About the self decays to nonisolated part.

nonisolated self initializers "decaying" the isolation

Can we make the access to the nonisolated self "wait" after the initialization finished?

For example:

actor Clicker {
  var count: Int
  func click() { self.count += 1 }

  init(bad: Void) {
    self.count = 0
    Task { await self.click() }
    self.click() 
    print(self.count)
  }
}

If we can postpone the await self.click() after init(bad:) is finished. There will be no race condition. And the print(self.count) prints 1.

We know that the task will be executed in future, now we guarantee that it is scheduled after the non-async init method.

The benefit is that you can always assume that all external / non-isolated accesses to the actor happens after the non-async init method is finished. It's easier to explain and easier to reason about.

Is this an alternative that already been considered? Am I missing something? Or there's an implementation limit?

1 Like
8

The answer is an overall yes: with SE-327, the compiler will no longer emit an error or warning about Sendable when initializing an actor's stored properties from a synchronous initializer.

EDIT: I made a mistake in my previous version of this reply. Currently the proposed rule is that there are three aspects that must be considered when it comes to Sendable and arguments to an initializer:

  1. The caller's isolation.
  2. The targeted initializer's status as being delegating or not.
  3. The targeted initializer's isolation to self

This complexity exists so that it's possible to pass non-Sendable values to a delegating initializer, but I think the rules around it are going to be too complex for people to understand. In addition,
by relying on whether an initializer is delegating or not, the rule accidentally re-introduces a need to distinguish delegating and non-delegating initializers in an API, i.e., we would need the convenience keyword again.

I think what we need for now is something simpler: when delegating from one initializer to another, passing non-Sendable values is OK. But to enter an initializer from the outside, the values must be Sendable. This matches up with how methods work for an actor, and should be intuitive while still maintaining safety.

9

One way to postpone the await self.click() as you describe is to sort of reserve, or lock, the actor at the start of the initializer, and then unlock it once exiting. This way, any new tasks that try to await during the initialization are only run afterwards. That idea was considered in an early pitch here (and discussed on the forums here) but I forgot to put it into the proposal. A solution like that would remove the race condition for plain actor types and would be quite nice.

Unfortunately, one of the downsides of that locking idea was that it didn't solve the problem for this kind of situation:

@MainActor
class MainActorClicker {
  var count: Int
  func click() { self.count += 1 }

  nonisolated init(bad: Void) {
    self.count = 0
    Task { await self.click() }
    self.count += 1
    print(self.count)
  }
}

Here, I've changed the actor type to instead be a MainActor-isolated class. Plus, I've added nonisolated on the init so that people can call MainActorClicker.init without an await. This in effect gives us an actor instance whose data and methods must be used while on the main thread.

The locking idea works if the operation to lock the executor is guaranteed to not block a thread. That's true for plain actor types, since the instance's executor was freshly created and is guaranteed to be idle. For the code example above it's not true, because another task may be using the MainActor when calling MainActorClicker.init. We would have to block the thread running the non-async MainActorClicker.init so it can wait for the MainActor to become free in order to reserve it during initialization. But, blocking threads goes against the design of Swift concurrency.

Imagine that, instead of trying to reserve the shared executor, we have each fresh instance of a MainActorClicker contain some special flag / lock to communicate to other tasks that the object is still being initialized, so its methods aren't safe to enter yet. In practice I do not think that is workable, since it would mean runtime tests for every method call to check if it is safe to enter, etc.

1 Like
10

Note: This is just an observation from trying to apply actors to my code, not a review. Also, sorry if I am creating inconvenient noise by bumping this old topic.

That would have been my intuition as well, and I ran into this problem when trying to apply actors to some really simple piece of code.

Actor type:

public actor PluginRegistry {
    
    public typealias Registrations = [String: () -> Any]
        
    private let registrations: Registrations
    private var pluginHandles: [String: AnyObject] = [:]
    
    public init(_ registrations: Registrations = [:]) {
        self.registrations = registrations
    }
    ...

Use site:

var registrations: PluginRegistry.Registrations = [:]
try PluginRegistry.append(to: &registrations, AdderPluginInterface.self) {
    return AdderPluginObject()
}
let registry = PluginRegistry(registrations)

gives (Xcode 14.3, Swift 5.8)

Non-sendable type 'PluginRegistry.Registrations' (aka 'Dictionary<String, () -> Any>') passed in call to nonisolated initializer 'init(_:)' cannot cross actor boundary

I see that SE-0327 contains examples where initializers call into asynchronous code, but I'm not doing any of that, it's just plain stored property initialization, and the initializer is called from synchronous code.

regards,
Karl