XCode 15b4 compiler changed behaviour (possible bug) with weak properties being cleared prematurely

Question

Created Jul ’23

Replies 5

Boosts 0

Participants 3

I've just spent a couple of hours zeroing in on a very weird bug. It seems that the Swift 5.9 compiler in XCode 15b4 is being overly-aggressive in clearing weak vars, compared to previous versions.

I have a method that returns an SKAction?.

In the debugger, I traced code and verified that the value it attempts to return is a

some : <SKMove: 0x280422b40>

However, the caller saves that value into a member which appears immediately after assignment nil

I tried adding a local variable for the return value, as seen below. That worked but

    override func perform() -> Bool {
        if cachedSKA == nil {
            let xcodeBugWorkaround = gameAction.makeSKAction()  // was assigning nil with xcode15b4
            cachedSKA = xcodeBugWorkaround
//  cachedSKA now shows in the debugger as non-nil
            if let activeScene = tgTouchgram.mostRecentlyStartedPlaying?.activeSKScene,
               let foundNode = referredNode(on: activeScene) {
                    cachedTarget = foundNode
            }
        }
// BUG - still fails at this point - cachedSKA appears nil again
        guard let ska = cachedSKA, let targetNode = cachedTarget else { return true }
        targetNode.run(ska)
        return true
    }

The problem appears to be because cachedSKA is a weak vars.

I am fairly certain it is being set to nil incorrectly.

Even though, within this method, the value is used, it is clearing the property as the sole holder of the object reference.

A fussy walkthrough reveals the compiler's changed behaviour is probably legal but certainly unexpected.

Working version, still keeping the property as weak:

    override func perform() -> Bool {
        var localSKA: SKAction? = cachedSKA
        if localSKA == nil {
            localSKA = gameAction.makeSKAction() 
            cachedSKA = localSKA
            if let activeScene = tgTouchgram.mostRecentlyStartedPlaying?.activeSKScene,
               let foundNode = referredNode(on: activeScene) {
                    cachedTarget = foundNode
            }
        }
        guard let ska = localSKA, let targetNode = cachedTarget else { return true }
        targetNode.run(ska)
        return true
    }

Answered by Scott in 758947022

Actually I think you are seeing correct behavior. The bug is in your code: it holds only a weak reference to the new object, so it is immediately eligible to get deallocated. It’s basically equivalent to this, which generates a compiler warning:

weak var weakRef = NSObject() // warning: instance will be immediately deallocated because variable 'weakRef' is 'weak'

In this simple example the compiler can issue the warning because it knows there are no strong references to the object. In your real code, the compiler can’t prove there aren’t any strong references being held somewhere else, so it can’t issue a warning.

The real question is why the code worked (not deallocating the object immediately) in previous versions. Assuming your own makeSKAction() method never had any side-effect of storing a strong reference to it somewhere, then I’d suspect the change is in SpriteKit. If the previous logic for creating an SKMove object kept a strong reference to it within SpriteKit with a lifetime any longer than your perform() method (maybe just in the autorelease pool, or maybe a long-lived cache) then your code would work. But if the implementation has changed to return an SKMove that is eligible for deallocation immediately if not retained, then the bug is revealed.

Boost

Answer 1

Scott OP

Jul ’23

Accepted Answer

Actually I think you are seeing correct behavior. The bug is in your code: it holds only a weak reference to the new object, so it is immediately eligible to get deallocated. It’s basically equivalent to this, which generates a compiler warning:

weak var weakRef = NSObject() // warning: instance will be immediately deallocated because variable 'weakRef' is 'weak'

In this simple example the compiler can issue the warning because it knows there are no strong references to the object. In your real code, the compiler can’t prove there aren’t any strong references being held somewhere else, so it can’t issue a warning.

The real question is why the code worked (not deallocating the object immediately) in previous versions. Assuming your own makeSKAction() method never had any side-effect of storing a strong reference to it somewhere, then I’d suspect the change is in SpriteKit. If the previous logic for creating an SKMove object kept a strong reference to it within SpriteKit with a lifetime any longer than your perform() method (maybe just in the autorelease pool, or maybe a long-lived cache) then your code would work. But if the implementation has changed to return an SKMove that is eligible for deallocation immediately if not retained, then the bug is revealed.

1

Answer 2

endecotp OP

Jul ’23

Scott is right. I thunk you misunderstand what weak variables are.

0

Answer 3

AndyTouchgram OP

Jul ’23

I don't misunderstand weak variables. I don't know if Scott's theory is right that it's a change in SKMove's allocation or if it was a compiler change to how ref counting and weak checks work.

The point I am making is that this is a change in behavior that's been there at least 2 years so is likely to catch people by surprise. This code hasn't been changed in a long time.

There's more detail I didn't go into earlier that made this even weirder to diagnose.

I have the same code running in a pure application and it does not immediately nil the weak reference there.

The only place where this behaviour appeared was running inside the context of an iMessage app extension.

0

Answer 4

Scott OP

Jul ’23

The point I am making is that this is a change in behavior

Definitely, but it would be a change of undefined behavior, unless there really is documentation of this type having different lifespan behavior than the norm. (But I’d doubt that.)

so is likely to catch people by surprise.

Yep, sounds like a case of getting bit by accidentally relying on undefined behavior without realizing it.

Just curious: in the old behavior, what was the lifespan of these objects that you never kept a strong reference to? Did they dealloc right after your perform() (which would defeat the attempt to cache them) or at some point much later? Was the caching actually effective?

0

Answer 5

AndyTouchgram OP

Jul ’23

Just curious: in the old behavior, what was the lifespan of these objects that you never kept a strong reference to? Did they dealloc right after your perform() (which would defeat the attempt to cache them) or at some point much later?

It's a good question and the short answer is probably caching didn't give much benefit.

SKActions are very commonly used in groups where the group action has a strong ref plus targetNode.run(ska) implies that the node is hanging onto the action for at least some running time. There can be repeated triggering via timers.

I wasn't evaluating when they disappeared and these 1 or 2 strong refs are why I was nervous about my declarative code retaining a strong ref.

As I explained above, this code's operating across domains.

It is very similar to UI framework code where you have your own objects in Swift (or C++) which map to low-level UI objects and cross-references can be dangerous sources of leaks.

I'm still thinking about making the ref weak again for safety and just using a local to fix this allocation.

My opinion hasn't changed on the legality of this behaviour - the guard at the bottom means it's certainly legal for the weak property to have been cleaned up before used, but it's a very surprising change (especially only in the extension).

0