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
I'm glad that this is getting tackled, it is a big hole in the system.
The proposal does a great job explaining the problems, that need to be solved. I'm curious about a few things:
Problem #1: the proposed solution and rules explained are complicated, and overly narrow. For example, it is perfectly fine for a sync init to touch nonisolated state on the actor once it is fully initialized.
In my opinion, the logical behavior we need to model is the "state switch" from being in the calling context's task/executor to the actor task/executor and provide a useful model for that. If we can do this clearly, the rules that are outlined all become emergent. I think you can go with a simpler set of behaviors:
In a sync init, "self" transitions from being a "some assembly still required" self to being a nonisolated Self: in a sync init, you aren't and can't be on the actors executor, so you're outside of the actor's isolation domain. It should be perfectly fine to pass this off to other functions that take a nonisolated reference to the actor's type - it is fully constructed after all. This allows you to touch its nonisolated state etc, by the existing type system rules.
In an async init you have several possible behaviors, which is a red flag that we have a design problem. We could follow the same policy as sync inits, and have an explicit syntax to switch to the actors executor and use await to mark the suspension point. We could make this implicit (if the implicit suspension is not a problem). We could even leave it as a nonisolated self for consistency with the sync case.
If you make the async case do a hop implicitly as the proposal indicates, then the only difference is that a sync initializer has type nonisolated Self after init, and the async case is isolated Self. I think that a simple rule like this would be easier to explain, and would be more useful (again, nonisolated state should be accessible in a sync init).
Problem 2: I have no problems eliminating global actor annotations on stored state, makes perfect sense.
How does the proposal handle stored class instances? This seems like a big hole that needs a solution:
actor SomeActor {
var str : NSMutableString
init() {
// memory safe because we know how NSMutableString works, but not safe in general since this could access task local values etc.
str = NSMutableString()
}
init(s2 : NSMutableString) { // totally fine since this isn't isolated, no sendable check.
str = s2 // totally not fine.
}
similarly, initializer syntax for non-sendable types needs to be determined.
I don't see anything in the proposal addressing this.
On problem 3, it is unfortunate that we require convenience marker on delegating inits; as the proposal indicates there is no reason for this, this probably fell out of the implementation accidentally?
I don't understand this:
and:
Two things:
I understand that we cannot remove the ability to use convenience for ABI reasons, but why would we continue requiring it for new types? I don't see how ABI compat applies to unwritten code. I would recommend just making it optional, keep it on existing code with ABI concerns, and make the abi checker tool handle it correctly. Rationale: this makes the language more consistent, cleans up code in iOS and other downstream apps which don't care about the same ABI issues that Apple does, and doesn't break apple's abi.
If we want "way to mark initializers that must delegate" then we should have the same affordance for structs and enums. I'd recommend splitting such a thing out to its own discussion, because it has dubious value and doesn't seem to warrant the language complexity. Similar things have been repeatedly proposed in the past (e.g. "must delegate to super") and we have consistently shot them down.
While it is OK for the synchronous init to touch nonisolated stored properties, that does not mean that it's safe for it to invoke a nonisolated method, since that method can create a task that captures the reference and begins running on another thread.
As of now, custom executors prevent the synchronous initializer (and deinit) from starting-off with actor isolation, because the executor for an exclusively-owned actor instance may be shared by other actors, and the executor may be busy during initialization (or deinitialization). We fundamentally cannot await to switch to that shared executor from a synchronous context, so we're left with the escaping-use restrictions detailed in the proposal.
I've been thinking about this "state switch" a bunch lately, but perhaps from a different angle than what you're suggesting. Describing the isolation of the synchronous init is hard, because its neither nonisolated nor actor-isolated. A synchronous init can maintain race-safety by enforcing exclusive ownership of the actor reference by the task that invoked the initializer, hence the escaping-use restriction. That restriction exists because we can't say that a particular actor instance is non-Sendable, as part of its type in the language.
But, we can say that an actor parameter has an isolation as part of its type. If we extend that more generally, I think one could describe the isolation of the actor reference in a synchronous init, even while it is in the "some assembly required" state. Imagine there is a third kind of isolation in the model, called task isolation, which says that exactly one task has a reference to the actor. That fact only guarantees safe access to the stored properties of the actor, because the actor's executor may be shared. In addition, that isolation status decays to being nonisolated after doing anything nonisolated, like passing it to a nonisolated method. If we were to model this imagined isolation's "decay" in SIL, we could then permit calls to nonisolated methods from a synchronous init:
actor A {
init(_ val: Int) {
self.x = val
if self.x == 0 {
self.x = 1
self.meth() // note: the nonisolated call
}
_ = self.x // error: cannot access stored properties on `self` after use in nonisolated call
Task { await self.iso() } // OK
}
nonisolated func meth() { /* ... */ }
func iso() { /* ... */ }
}
Users would just need to give up the capability to access stored properties after that call. This solves all of the issues in the "flow-sensitive isolation" described in the Alternatives Considered proposal. In particular, we now have a specific expression that causes the isolation to change.
I do plan to investigate this "decaying" isolation idea further, but did not include it in this revision of the proposal. Anyone have thoughts on whether it should be included?
I don't quite understand the danger with the init() above. Why does access to task-local values make this unsafe? Is it because one can sneak a non-Sendable value past Sendable checking on the arguments using a task-local value?
The fact that a discussion of Sendable-ness and what is permitted in a deinit is missing from the proposal is my mistake. When restructuring the proposal text to include the removal of global-actor isolation of stored properties, I forgot to merge in the remaining parts. Here is a summary of what I meant to convey about those topics:
Sendable-ness
The arguments to a non-delegating actor initializer must conform to Sendable. Delegating initializers can recieve non-Sendable types.
The reasoning here is that wrapping an existing class instance with an actor is not safe, since that reference may not be exclusive to the actor. Combined with the other rules about Sendable parameters and return values for actors, this means that reassigning a stored property of a non-Sendable type, like in init(s2:) above, should be safe.
Deinit
A deinit is in a very similar circumstance as an initializer: a deinit starts-off with exclusive access to the actor instance within a synchronous context, and must have access to its stored properties. A deinit cannot be async, because it can be invoked from anywhere. The idea of creating a new task to invoke the body of the deinit is covered in the Alternatives Considered.
Because of custom executors, exclusive access to the instance does not imply that we can freely invoke actor-isolated methods on the instance member. A MainActor-isolated class is one example of this. If the deinit were permitted to invoke one of the class's isolated methods without being on the executor, then a race can happen.
Just like a synchronous init, if we allow the actor self to be captured by a task, then a race can be observed in the deinit. Applying the escaping-use restriction to a deinit would solve this race. On the downside, that restriction would also prevent the capture of the actor instance in a task, so isolated methods cannot be invoked at all during or after a deinit. Programmers would need to pass the individual pieces of the actor instance's state to a nonisolated function that performs the tear-down, in order to share the tear-down code between the deinit and an isolated method. For now, I propose that the escaping-use restriction is applied to a deinit in order to make it race-free. We can later ease this restriction with better analysis (i.e., the "decay" of isolation I mentioned earlier), to enable task capture.
Here's an example of the kind of tear-down function that would be needed:
class Connection {}
actor A {
var connections: [Connection] = []
/* ... */
func shutdown() {
A.shutdown(self.connections)
}
deinit {
A.shutdown(self.connections)
}
static func shutdown(connections: [Connection]) {
for connection in connections {
// do shutdown for `connection`
}
}
}
Convenience inits became repurposed as a type of initializer that is truly nonisolated all the way through, as a way to workaround the escaping-use restrictions. Based on your suggestions, and after further investigation, I also learned that we can workaround any ABI breaks that I imagined, because there are no sub-actors that directly call the non-allocating entry-point of an actor's non-delegating initializer.
Here's summary of what I think we should change in the proposal:
Delegating Initializers
Like structs and enums, an actor's delegating initializer does not require convenience, but if it does delegate, then it must delegate on all paths from the start of the initializer. A delegating initializer can be marked as nonisolated (or isolated to a global-actor), but does not have to be. If a delegating initializer is not marked as nonisolated, then the same rules that would apply had it been non-delegating, will also apply to the initializer.
Furthermore, a non-delegating initializer cannot be marked nonisolated. Reason: it really doesn't make sense for any non-delegating init to be nonisolated, because the under-construction instance's stored properties must be accessible without an await. Having nonisolated act differently only for initializers is inconsistent.
I thought more about how we could model this state switch explicitly in the language, especially to make the switch over to nonisolated within a synchronous init make sense to users. The "decaying" isolation I mentioned in my first reply relied on a particular nonisolated use of self to mark the beginning of that region in the initializer, but perhaps that idea can be pushed further.
Let's suppose that the actor's self starts off with an uninitialized isolation kind at the start of any actor's initializer. While the isolation is uninitialized, so too is self. As with any type, while self is uninitialized, only access to its stored properties are permitted. In order to do anything else with self, users must "assign" or somehow ascribe an isolation to self exactly once along every control-flow path (like a let-bound stored property):
actor A {
var x: Int
init(sync val: Int) {
self.x = val
if val < 0 {
self.x = 0
self = nonisolated(Self) // ascribe "nonisolated" isolation to `self`
self.x += -val // error: `self` is nonisolated at this point
}
// error: missing isolation before return from non-delegating init
}
init(async val: Int) async {
self.x = val
self = await isolated(Self) // gain access to the executor
// this is currently done implicitly.
}
init(async2 val: Int) async {
self.x = val
self = nonisolated(Self)
_ = self.x // error: missing `await` during property access on nonisolated self.
}
// complication: should the ascribed isolation be carried
// through delegating inits?
init(delegating val: Int) async {
if val > 0 {
self.init(sync: val) // returns nonisolated self
} else {
await self.init(async: val) // returns isolated self
}
// error: `self` has conflicting isolation here
}
}
For any mismatches in isolation at control-flow joins, such as in the complication above, we could just take the minimum of the set of incoming isolations and say that self is treated as nonisolated... that is, anything + nonisolated = nonisolated.
I don't know how much of this makes sense, but I wanted to put the idea out there for comments.
Thinking more about it, I think it is helpful to consider the case of distributed actors when defining this model. Given the goal to support distributed actors, I think the only "possible to reason about" model is that Sendable checks are done when invoking the initializer: This corresponds to transporting the sendable values across the wire to construct the remote object.
Given that, the actor initialization is logically a (potentially failable) operation within the new context. This means it shouldn't have access to any TLVs or global variables from the invoking context until fully initialized.
Once fully initialized, if async, it seems like it should transparently hop over to actors's executor (which of course, with custom executors could be shared, so sync may be needed which is why this can only happen if the init is async). All distributed actor inits will probably need to be async, so this covers that case.
The only uncovered case is the sync actor case. When fully initialized, I think the correct way to model this is with self being a nonisolated actor reference.
The current formulation of distributed actors does not allow for the direct allocation and initialization of a new remote object in another process. There are only two ways to create a distributed actor instance: local initialization, by invoking an init on the type, or remote resolution, by invoking the static resolve function belonging to the type:
Using an init to represent this resolve step was considered. For example, saying that all distributed actors have a synthesized init?(id identity: Identity, using transport: ActorTransport) throws that cannot be delegated to and users are not allowed to implement themselves. But, this init felt like it went too far against the grain of the existing implementation for inits. So, rather than make a new initializer kind for distributed actors in the compiler, we landed on the synthesized, static resolve function. This function also helps make it clear to users that they're not getting back an an instance with the default values for its stored properties:
distributed actor Person {
var name = "Hannah"
}
let p = Person(id: id, transport: transport)
// p.name may not be "Hannah"... it may have already existed!
Remote resolution of an instance does not necessarily fail because the instance corresponding to that identity does not exist. Instead, all other interactions with a distributed actor, from outside of its isolation domain, are implicitly async and throwing. Not only is this implicit throwing needed to handle connection failures, but presumably, this allows the remote process to "passivate" the instance when not actively used, or initialize it only upon the first method call, etc. So, I don't think distributed or regular actor initializers need to be implicitly throwing or failable.
Staying within the current distributed actors model, the only way to allocate a distributed actor that exists "remotely" is to pass the values needed to initialize it through a method call to some existing distributed actor instance, and having that method return the identity of the actor. Since those values must be Sendable in order to be passed to a distributed method, we will effectively have a corresponding Sendable requirement on inputs during initialization in all cases, whether the actor is distributed or ordinary.
Contrary to what is currently in the proposal, I agree with there being some point at which self becomes a nonisolated actor reference in a synchronous init. It's ultimately necessary to support distributed actors too (the transport.actorReady(self) call has to happen in a synchronous init too). But, using the point at which self is fully initialized is problematic. In short, one of the problems is that, while it's OK for a write to a stored property to happen regardless of whether self has become fully initialized, that's not the case for isolation. Once you cross over to becoming nonisolated, you cannot go back. I need to think more about how to precisely describe this point and simplify it for users to help them understand.
Thanks for thinking about distributed actors here, Chris!
As Kavon outlined (thanks! Sorry, I was having such a slow Monday...) we really do not need (or want) "remote initializers" but the systems are built around the concept of resolving remotes. An initializer is always local; this is important and powerful because it allows us to have such actor contain things which only could exist locally -- you can ask a remote "Boss" to get you a "Worker", and the boss can pass non-serializable values to the from itself to the worker, and return to you a reference to that worker etc.
This is also what enables advanced "virtual" actor patterns (popularized by orleans), where you don't ever really know if the remote actor is alive or not at the moment of resolve, but the implementations either give you back the existing one, or could even create a "fresh" one for it -- this relates to the passivation patterns Kavon mentions
Summing up: distributed actor inits are indeed tricky, but because of the injection of the transport.actorReady(self) call, not because any remoteness of them.
I'd love to catch up in detail by the way, hope to set up something soon
I need to write up a review here about the proposed semantics and the calls we need to inject in these inits, but I worked with Kavon and we're confident the two mesh well together. We didn't want to muddy this proposal with a lot of details about distributed init "actor ready" call injection though -- it'll fit the rules though, so we're fine
In the last paragraph of "Initializer Races", the authors said:
The existing implementation makes it impossible to write a correct init for the example above, because the init is considered to be entirely isolated to the @MainActor . Thus, it's not possible to initialize self.statusat all . It's not possible to await and hop to self 's executor to perform an assignment to self.status , because self is not a fully-initialized actor-instance yet!
I don't understand why we can't initialize self.status. Can there be more detailed explanation on this?
The "existing implementation" that the proposal is referring to is quite old now (the proposal was originally started before Swift 5.5 released). Previously, the typechecker would prevent access to self.status because it is considered isolated state for an actor instance, which is not the same as the MainActor. Given that it's a synchronous initializer, there was no way to gain isolation either. It's still debatable whether global-actor isolation should actually be permitted on an actor's initializer. I didn't see any downsides to simply permitting the access. The fix was simple, had no downsides I could think of, and if programmers actually need to access MainActor-isolated state when initializing an actor, the workarounds are annoying:
After reading through the proposal a second time, from my view the new rules for problem 1 (initializer data races) and problem 3 (initializer delegation) are overly complicated. Two main points I am thinking:
The proposal tries to interweave sync/async semantics with isolated/nonisolated traits of initializers. While we can write down the details in some diagrams, I am not sure if it is worth for engineers to understand and memorize these.
While there may be some concern about ABI or source-level compatibility (I am not sure at this), this proposal tries to unify the initializer delegating rules in the actor world. But the convenience keyword from classes seems to put limit on our design.
Here is my modified version for problem 1 (initializer data races) and problem 3 (initializer delegation):
sync/async trait don't have any restriction on an actor's initializer. Whether an actor's initializer is async only means whether it may give up the current thread (same as a normal function or method).
There are only two types of initializer in actor: isolated init() and nonisolated init().
In the body of an isolated init(), the initializer is either isolated to the actor's internal queue, or it is isolated to a global actor's queue (if there is some annotation like @MainActor).
We can only initialize stored properties in an isolated init(), while calling other methods of the actor is prohibited.
In the body of a nonisolated init(), we can not initialize any stored properties. We must call isolated init() to do the stored properties initialization.
In the body of a nonisolated init(), we can call any other methods of the actor while treating self as nonisolated self.
As for the convenience keyword, my suggestion is to treat it as an alias for nonisolated. We can deprecate convenience in actors in later Xcode versions.
Can someone explain to me what it would mean if global actors are generally disallowed on stored properties for the following examples?
@MainActor
struct S {
var value: Int // implicitly @MainActor
}
@MainActor
class C {
var value: Int // implicitly @MainActor
}
// The current proposal mentions:
// "The global actors proposal explicitly excludes actor types from
// having stored properties that are global-actor isolated. "
//
// From the global actors proposal:
// "An actor type cannot have a global actor attribute. Stored instance
// properties of actor types cannot have global actor attributes."
// This does not seem to be enforced though?! 🤨
actor A {
@MainActor // does COMPILE in Swift 5.5
var value: Int
...
}
The explicit global actor protection on an actor stored property would be disallowed, fine it'll be protected by the actor itself. What about other types, what's going to protect the properties if the implicit actor is gone?
Last but not least, was it ever decided that actors will never gain inheritance? Not that I want/need that feature, but it feels a bit odd trying to eliminate convenience keyword on actors when the answer to that question might be undecided. If we'd ever introduce inheritance to actors, wouldn't they need convenience to have similar but complex delegation rules like classes?
I think the rule we really need here is that a stored property has the same isolation as its enclosing type: a stored property of an actor is isolated to that actor, a stored property of class is isolated to the global actor of that class (if it has one) or has no isolation.
Actors do not have inheritance today. If they were to gain inheritance in the future, we would not necessary have initializer inheritance, which came from Objective-C and is the root of the complex delegation patterns. Without initializer inheritance, the designated/convenience split is unnecessary and the whole model becomes simpler. So, I would prefer that we eliminate convenience from actors, and leave it as an oddity for classes.
Thanks everyone for the productive discussion here! We're returning this proposal for revision so the authors can explore the "flow-sensitive" semantic model.
While I don't disagree, what about value types without value semantics? It feels like that those are made unsafe. Sure you could argue that the entire object types they care around need actor protection, but what about the case where only the stored properties on the value type need actor protection but not the entire class as it might need to remain unsafe in other places for whatever design reasons?! I know this a very muddy question, but I don't think it's that unreasonable.
I think you're arguing that on a type that needs actor protection, there might be some stored properties that don't need that protection, e.g., because they use some other synchronization mechanism. If that's so, I agree---but I think it should be a specific opt-out that's clearly unsafe/unchecked, e.g.,
@MainActor class C {
nonisolated(unchecked) myLockedState: ManagedBuffer<...>
}
// not actor protected by design
// may not even be in our control (e.g. 3rd party class)
class C { ... }
// no value semantics by design
@MainActor
struct S {
// no implicit actor protection after this proposal
// unless I still misunderstand something
let c: C = ...
func foo() {} // implicit MainActor
}
Can you clarify on that scenario please?
I think there are a few options but they seem to be somewhat forced on the developer:
make the struct obey value semantics (may or may not be possible or even desirable)
leave it in an seemingly, implicit unsafe state
search for a different solution
Do we really need the "no global actor protection on value type's stored properties"? There is no deinit in that case, at least nothing from the developers standpoint. It feels like this rule punches a hole here and creates such a strange edge case.
I don't think anything ever said "no global actor protection on stored properties". The point is that a stored property of a type should not be on a different actor from the type itself.
S is @MainActor. c is a stored property of S, so it can only be accessed on the main actor. The fact that C is not Sendable means that you won't be able to take the value from c and pass it into another task or to another actor.
That clears it for me, however I think the wording in the proposal got me slightly confused:
In summary, the most straightforward solution to the problems described earlier is: global-actor isolation should not apply to the stored properties appearing within any nominal type.
So in short besides actors themself (they'll protect their stored properties), any other nominal type will have its stored properties protected by a global actor (if any), which also happens to protect the whole type.
If that's somewhat correct, can we potentially relax this rule and just say that on any nominal type every stored property must be protected by one and the same actor (if any).
on an actor this is trivial
on other types the stored properties either implicitly gain protection from the actor which protects the entire type, or if the type has no explicit global actor applied to it, all stored properties must share the same global actor or none
// For the last example.
// Would that be still valid protection?
// No explicit actor protection on the type itself.
struct T {
@MainActor
var x: Int
@MainActor
var y: Int
@MyActor
var somethingElse: Foo { /* computed property */ }
}
Don't get me wrong, I'm all supporting unifying and simplifying the rules on actors and the rest of the concurrency. ;) It's still such a beast to tame.
actor MyActor {
@MainActor weak var delegate: MyActorDelegate?
…
}
I use this pattern in my app so the UI can set the delegate, and all the callbacks happen on the main actor, but processing doesn’t block the main thread.
