Factory breaking change under consideration....

[hmlongco]

Issue #111 describes new warnings that can occur when Swift strict-concurrency is set to complete in your project.

On consideration of the various ramifications, I may need to consider a breaking change to Factory to do this properly. The current definition of SharedContainer is...

public protocol SharedContainer: ManagedContainer {
    static var shared: Self { get }
}

But I may need to change that to...

public protocol SharedContainer: ManagedContainer {
    static var _container: Self { get set }
}

extension SharedContainer {
    /// Defines a thread safe access mechanism to the shared container type.
    public static var shared: Self {
        get {
            defer { globalRecursiveLock.unlock() }
            globalRecursiveLock.lock()
            return _container
        }
        set {
            defer { globalRecursiveLock.unlock() }
            globalRecursiveLock.lock()
            _container = newValue
        }
    }
}

With access to the static shared container managed by access functions. From the outside and in most cases it looks the same...

let service = Container.shared.myService()

With the only issue occurring if people define their own container types.

public final class MyCustomContainer: SharedContainer {
    /// Define the default shared container.
    public static var _container = MyCustomContainer() // NAME CHANGE NEEDED
    /// Define the container's manager.
    public var manager: ContainerManager = ContainerManager()
    /// Public initializer
    public init() {}
}

Any thoughts?

[o-nnerb]

Interest issue you are discussing. In my view, I don't think this approach is the most correct.

I was reviewing the Factory code to better understand the bug involving strict-concurrency, and there are two points that can serve as a starting point to address this warning.

The first one is that "shared" should be a "let" property.

The second point is that the Factory needs a PR that implements conformance to Sendable. Even though the code is thread-safe due to the use of locks, Swift is not able to know that at compile time without the use of Sendable.

I believe that by making these improvements and using a constant for "shared," we will have a solution to this warning.

What do you think?

[hmlongco]

Ummm... just tried that and it eliminated the majority of the warnings. My biggest problem with that approach was in the fact that for testing and whatnot your couldn't "reset" a shared factory by simply assigning a new one to it.

Those doing so, however, could switch those to simply use a reset...

// Instead of...
Container.shared = Container()
// Do...
Container.shared.reset()

Will noodle some more...

[o-nnerb]

Hmmm, I understand what you're trying to say. Well, I don't think I agree with the idea of being able to update the value of "shared". However, since it's included in the documentation and is a feature of the Factory, I think your initial solution will be the best approach.

As I'm a beginner in using the Factory, I ended up confusing the "reset" method of the manager with your suggestion for the Container. Indeed, it would be strange to have a "reset" method to reproduce the same behavior already implemented for the manager.

Would the idea be to implement a reset like this?

extension Container {

    func reset() {
        Container.shared = Container()
    }
}

Anyway, the concurrency problem wouldn't be solved. Because the warning is related to the property being a var and not a let. So we go back to your initial proposal as the best solution to the problem. 🤔

[o-nnerb]

And, it's worth mentioning that changing "shared" to "let" would also introduce breaking changes. 🫠

[hmlongco]

Both are breaking changes. The first approach affects those who've created custom containers, the second those who may have done the shared reassignment.

That said, I think the let approach is best, as that's easiest to find and replace and doesn't let people bypass the mechanism by accessing "container" directly.

Factory Changelog

2.2.0

• Converts "shared" container variables from var to let. Issue - Concurrency-safe warnings when strict-concurrency is set to complete #111

• Add convenience reset() function to container. Issue - Concurrency-safe warnings when strict-concurrency is set to complete #111

  Note that issue Concurrency-safe warnings when strict-concurrency is set to complete #111 could be a breaking change if your code attempts to "replace" a shared container. Do reset instead.

  Container.shared.reset() // Do this...
  Container.shared = Container() // Not this...

[JeroenBakker-wbd]

Nice one, I also think using a let makes more sense as you are already sure it is thread safe, we can now tell the compiler it is thread safe.

Using the reset method is a nice way to achieve what you want to do, and keeps mutation inside its own class (which can be tested to be thread safe) rather than outside. Thanks for your quick reply and experiments

Will this be a 3.0 release as it is breaking?

[hmlongco]

Major.Minor.Bug

Factory 2.0 was a major change/rewrite, Factory 2.2 will be a minor change that "could" break some code, depending on usage.