Isolation and subclassing

Evolution Discussion
1

I've got back to working on isolated/async deinit, and after rebasing I've got new warnings in some of my unit tests, which seem to originate from this commit [Concurrency] Don't allow subclasses of nonisolated classes to add gl… · apple/swift@c9bfe8c · GitHub by @hborla.

Adding global actor isolation via subclassing admits data races because
actor-isolated types are Sendable while nonisolated classes are not (unless
otherwise annotated), so this allowed bypassing Sendable checking.

I can think of the following scenario of how checking could have been bypassed:

class A {
    var x: Int = 0
}
@FirstActor class B: A {
    var y: Int = 0
}

@FirstActor func f() async {
    let b = B()
    b.y += 1 // ok
    b.x += 1 // ok, accessing A<@FirstActor>.x in the context of @FirstActor
    let a: A = b // ok, casting to A<@FirstActor>
    a.x += 1 // ok, accessing A<@FirstActor>.x in the context of @FirstActor
    await g(b)
}

@SecondActor func g(_ b: B) {
    b.y += 1 // error
    b.x += 1 // ok, but should be an error - accessing A<@FirstActor>.x in the context of @SecondActor
    let a: A = b // ok, but should be an error - B inherits from A<@FirstActor>, but casting to  A<@SecondActor>
    a.x += 1 // ok
}

Banning adding isolation in the subclasses solves the issue, but I think it is unnecessarily restrictive. I think it can be loosened.

The way I understand non-sendable classes, is that they are generic over isolation context. When type A is used inside actor-isolated context, it behaves as if it is isolated to that actor. When type A is used inside nonisolated async function - it behaves as if it is isolated to the current task. And that is what makes type A non-sendable - you cannot assign A<@FirstActor> to A<@SecondActor>.

But when isolated subclass inherits from non-sendable superclass, it actually inherits from the specialisations of the superclass. When accessing members of A through instance of B, they should be treated as isolated to B's isolation, even though A is not isolated. And in the context of the @SecondActor func g() type A means A<@SecondActor>, which is not the base class of B. So upcasting from B to A should fail.

On the other hand, there seems to be still uncovered hole with upcasting to AnyObject. For some reason the following code does not produce errors:

@FirstActor
func foo() async {
  let x: AnyObject = A() // A<@FirstActor>
  await bar(x)
}

@SecondActor
func bar(_ obj: AnyObject) {
  let y: A = obj as! A // A<@SecondActor>
  print(y)
}
1 Like
2

It's not possible to treat inherited properties as if they had been annotated as isolated in the subclass, because the compiler cannot see all accesses to that property. For example:

class NotIsolated {
  var value = 0
  @MainActor func doSomething() {
    value += 1
    // access some other MainActor-isolated stuff
  }
}

@MyGlobalActor
class AddsIsolation: NotIsolated {
  func doSomething() {
    // This is fine, 'value' is not isolated
    value += 1
  }
}

@MyGlobalActor func onGlobal() {
  let c = AddsIsolation()
  Task { @MainActor in
    // This is fine because 'c' is 'Sendable' and 'doSomething' is isolated to MainActor
    c.doSomething()
  }

  c.value += 10 // oh no! races with above
}

That said, I agree that the diagnostic I added is too restrictive. I think the fix is to require that the subclass be non-Sendable. Usually, global actor isolation implies Sendable, but that's obviously not safe to do here. Even if we make that change, though, these patterns still can't really be made very ergonomic, because overridden methods in the isolated subclass must match the isolation of the superclass, meaning that most overrides will be nonisolated and cannot touch any isolated state added by the subclass.

This isn't how non-Sendable classes work. I can see why people had this mental model prior to SE-0414, because you had the assumption that a non-Sendable class can never escape the isolation domain that it's formed in, so it effectively behaved as if it was isolated to that concurrency domain. SE-0414 breaks this assumption in order to lift Sendable restrictions in many other ways that are extremely desirable. You still, of course, get the assumption that a non-Sendable class can only be used by one isolation domain at a time, but the value can be transferred between isolation domains.

3

I think SE-0414 generalises this model, but does not invalidate it. And according to SE-0414, diagnostic should be loosened even further - error should be moved from upcasting to actual usage of a in g:

@SecondActor func g(_ b: B) {
    // Regions: [(b, @FirstActor)]
    b.y += 1 // error
    b.x += 1 // error
    let a: A = b // ok, under SE-0414, because now we can track that A is isolated to  @FirstActor-isolated region
    // Regions: [(a, b, @FirstActor)]
    a.x += 1 // error
}

I think in your example, overriding doSomething() should give an error - it is isolated to @MainActor in the base class, but to @MyGlobalActor in the derived actor.

If we remove overriding, then under SE-0414 we get the following:

class NotIsolated {
  var value = 0
  @MainActor func doSomething() {
    value += 1
    // access some other MainActor-isolated stuff
  }
}

@MyGlobalActor
class AddsIsolation: NotIsolated {
  func doSomething() {
    // This is fine, 'value' is not isolated
    value += 1
  }
}

@MyGlobalActor func onGlobal() {
  let base = NotIsolated()
  // Regions: [(base)]
  base.value += 1 // ok
  await base.doSomething()
  // Regions: [(base, @MainActor)]
  base.value += 1 // error

  let c = AddsIsolation()
  // Regions: [(base, @MainActor), (c, @ MyGlobalActor)]
  Task { @MainActor in
    // This is fine because 'c' is 'Sendable' 
    print(c)
    // Regions: [(c, @ MyGlobalActor)]
    c.doSomething() // error: Cannot merge (c, @ MyGlobalActor) and (self, @MainActor)
  }
  c.value += 10 // ok
}

Adding actor isolation in the subclasses is ok, as long upcasted reference remains in the region associated with the actor of the derived class.

4

It was meant to just be an overload, but it's not necessary to illustrate my point:

class NotIsolated {
  var value = 0
  @MainActor func doSomething() {
    value += 1
    // access some other MainActor-isolated stuff
  }
}

@MyGlobalActor
class AddsIsolation: NotIsolated {
}

@MyGlobalActor func onGlobal() {
  let c = AddsIsolation()
  Task { @MainActor in
    // This is fine because 'c' is 'Sendable' and 'doSomething' is isolated to MainActor
    c.doSomething()
  }

  c.value += 10 // oh no! races with above
}
5 
// This is fine because 'c' is 'Sendable' and 'doSomething' is isolated to MainActor
c.doSomething()

Not exactly.

c is Sendable and can be passed to the another task - correct. I've used print(c) to illustrate that.

doSomething is isolated to MainActor and we are in a MainActor-isolated context, so we can call doSomething().

But we cannot call doSomething() passing c for self. Because doSomething() requires self and @MainActor to be in the same region, while c is already in the same region as @MyGlobalActor.

1 Like
6

I'm getting pretty confused about whether you're discussing today's concurrency rules, or the hypothetical rules that have been suggested above in your comments and mine.

Isolation regions only apply to non-Sendable values. Under today's concurrency rules, global actor isolation on a nominal type always implies a conformance to Sendable, meaning they can always be passed across isolation boundaries. Isolation regions do not have any impact on calling a synchronous method once you're already in an actor-isolated context, so if we literally just reverted my change, my example compiles warning-free under -strict-concurrency=complete with and without RegionBasedIsolation enabled (given that I add a declaration of @globalActor actor GlobalActor { ... }.

I believe the fix is to make global actor isolation require that a nominal type is not Sendable if it inherits from a non-isolated, non-Sendable superclass. That would make the example invalid at the point of capturing c in the Task.init closure, which is a @Sendable closure. If we later make Task.init take a transferring closure (in the future directions of SE-0414), it would make c.value += 10 invalid after the point of transfer.

1 Like
7

Yes, you are right. I guess that's why my explanation was confusing. Let me correct myself.

There should not be a region for c of type AddsIsolation, but region appears when it is upcasted to NotIsolated. And for correct type checking this region should remain associated with @MyGlobalActor.

@MyGlobalActor func onGlobal() {
  let c = AddsIsolation()
  // Regions: [], c does not need one
  Task { @MainActor in
    // c is Sendable and can be transferred
    // Regions: [], c does not need one
    let base: NotIsolated = c
    // Regions: [(base, @MyGlobalActor)]
    // Cannot call doSomething because (base, @MyGlobalActor) and (self, @MainActor) cannot be merged
    base.doSomething() // error

    // Upcasted value is implicit, but should behave exactly as above
    c.doSomething() // error

   let another = NotIsolated()
   // Regions: [(base, @MyGlobalActor), (another)]
   another.doSomething() // ok
   // Regions: [(base, @MyGlobalActor), (another, @MainActor)]
  }
  c.value += 10 // ok
}