mattmassicotte / ConcurrencyRecipes

Practical solutions to problems with Swift Concurrency
BSD 3-Clause "New" or "Revised" License
1.07k stars 33 forks source link

Recipe for making non-Sendable, mutable, properties be Sendable and immutable #10

Open levibostian opened 4 months ago

levibostian commented 4 months ago

I am quite new to Swift concurrency so I greatly appreciate the wisdom of others in this project to help me determine if this is a valid recipe or not. Any feedback is welcome!

A Xcode compiler warning that I receive a lot is:

Stored property '' of 'Sendable'-conforming class '' is mutable

Example of code that would give me this warning:

final class Foo: Sendable {
  var bar: String = "" <--- property would receive this warning 

  // To make things even more interesting, I may have a non-Sendable type that can 
  // give this warning as well as other warnings. 
  var bar2: NonSendableType = ....
}

// Structs give this warning, too. 
struct FooStruct: Sendable {
  var bar: String = "" <-- also gives this warning 
}

Because I saw this warning so much in my project, I wanted to see if there was a way that I could resolve all these warnings using a common design pattern while not having to use @unchecked Sendable for my data structure.

This is the "recipe" that I came up with.

First, create this class that acts as a wrapper around a generic data type:

public final class MutableSendable<DataType: Any>: @unchecked Sendable {

    private var value: DataType

    init(_ value: DataType) {
        self.value = value
    }

    private let _lock = NSRecursiveLock()

    func get() -> DataType {
        _lock.lock()

        let returnValue = value

        _lock.unlock()

        return returnValue
    }

    func set(_ modifyBlock: (DataType) -> DataType) {
        _lock.lock()
        defer { _lock.unlock() }

        self.value = modifyBlock(value)
    }

    func set(_ newValue: DataType) {
        _lock.lock()
        defer { _lock.unlock() }

        self.value = newValue
    }
}

Then, taking the code I gave earlier in this issue, I can apply this refactor:

final class Foo: Sendable {
  let bar = MutableSendable<String>(value: "") <-- no more warning! 

  // No warning for non-Sendable either! 
  let bar2 = MutableSendable<NonSendableType>(...)
}

// No more warning for structs, too. 
struct FooStruct: Sendable {
  let bar = MutableSendable<String>(value: "") <-- also gives this warning 
}

The warnings are gone, yes, but at a cost. You have to change the way you get/set the property in your class:

final class Foo: Sendable {
  let bar = MutableSendable<String>(value: "") <-- no more warning! 

  func modifyBar(newValue: String) {
    // I want to use: 
    bar = newValue

    // But I have to use: 
    bar.set(newValue)
  }
}

I would love to use something like a property wrapper to bring back the get/set syntax but the original xcode warning would still exist:

final class Foo: Sendable {
  @MutableSendable var bar: String = "" <-- 😞 warning comes back, property wrappers must use 'var'

  func modifyBar(newValue: String) {
    // But, I can use the nice get/set syntax! 
    bar = newValue
  }
}

I think recipes to resolve this compiler warning is missing from this project. Any interest in contributions for recipes for this warning?

mattmassicotte commented 4 months ago

It is really common to try to use locked wrappers for these kinds of problems.

https://mastodon.social/@cocoaphony/112134000958953244

I think it may be worthwhile to include something like this. However, in my own code I have migrated towards other approaches. Here's what I wrote in response to that post:

I think it is correct.

On my journey, I did stuff like this. And over time, I began to focus more on minimizing the need to cross boundaries, instead of making it easier. It can be a lot easier said than done though.

So while I am very into suggesting this as an option, I think the default for many is "I just want this to be Sendable" and I think that can be a trap. Now, like I said, just because I think it is a bad idea, doesn't make it any easier to do something else. Expanding actor boundaries can require redesigning your system, and that is often just infeasible.