Trying to understand an unowned reference crash

The tree class below keeps an optional reference to a node's parent. To break a strong reference cycle, I initially made that a weak reference. Upon seeing HeapObjectSideTableEntry at the top of my time profiles, I found a 2x performance improvement turning parent into an unowned pointer (mistakenly thinking that child nodes never outlive their parents). I then encounter a crash in Expr.__deallocating_deinit which I'm trying to see if I understand correctly. The crash occurs only when building optimized, and only with unowned parent. The crash does not occur using weak parent or unowned(unsafe) parent. Note that the sample code assigned to parent, but never uses it. Questions:

  1. Is the mere presence of an unowned reference to an object whose strong reference count has gone to zero a bug? i.e. Is this crash expected Swift behavior?
  2. Does making parent unowned(unsafe) actually fix the underlying bug here or does it just turn it into a silent memory corruption that will be much harder to track down? i.e. Is it safe to have an unowned(unsafe) reference to an object whose strong reference count has gone to zero without using it?
  3. Is my interpretation of what is happening here correct? Does the analysis below of the strong and unowned reference count look right?
  4. Is something else entirely here causing the crash?

The code below starts with a tree representing the sum of two vectors: [x, 1] + [y + 2, 1].
Running it unoptimized will add the vector components to produce [x + y + 2, 1 + 1].
Running it optimized will crash with

UnownedReferenceCrash(95477,0x1000d3d40) malloc: Non-aligned pointer 0x16fdff138 being freed

Here is what I think is happening.

  • Initially, y+2 has a 1 strong reference as a child of the vector, and 2 unowned references from its child nodes y and 2.
  • In addVectors, a[0].add(onRight: b[0]).flatten(), b[0] creates a 2nd strong reference to y+2
  • In add(onRight:), (shallowCopy + term) creates a temporary Expr node x+(y+2)
  • That adds a 3rd strong reference to y+2, and changes its parent to point to temporary
  • flatten() changes the temporary to x+y+2
  • that releases one of y+2's strong references since the temporary now directly points to its children
  • that also removed both of the unowned references to y+2 since y and 2's parents are now the temporary
  • replace(with:) copies the contents of the temporary into the first term of the first vector
  • returning from add(onRight:) releases the strong reference to y+2 from it's term parameter
  • returning from add(onRight:) releases that temporary Expr which y+2's parent still points to
  • when addVector's b goes out of scope, that release the last thing to hold a strong reference to y+2
  • y+2 deinits, while still holding an unowned reference to that temporary
import Foundation

final class Expr: CustomStringConvertible, ExpressibleByStringLiteral {
    unowned var parent: Expr? = nil
    var op: String
    var args: [Expr] = [] { didSet { setArgParents() } }
    func setArgParents() { for arg in args { arg.parent = self } }
    subscript(index:Int) -> Expr { args[index] }
    var count: Int { args.count }

    init(_ op: String, _ args: [Expr]) {
        self.op   = op
        self.args = args
        setArgParents()
    }
    convenience init(stringLiteral s: String) { self.init(s) }
    convenience init(_ op: String, _ args:Expr...) { self.init(op, args) }

    var shallowCopy: Expr { Expr(op, args) }
    var description: String { args.isEmpty ? "\(op)" : "\(op)(\(args.map{$0.description}.joined(separator: ",")))" }

    func addVectors() {
        let a = args[0], b = args[1]
        args.remove(at: 1)
        for j in 0 ..< a.count {
            a[j].add(onRight: b[j]).flatten()
        }
        if count == 1 { replace(with: args[0]) }
    }

    @discardableResult func replace(with expr: Expr) -> Expr {
        op   = expr.op
        args = expr.args
        return self
        }

    @discardableResult func add(onRight term: Expr) -> Expr {
        replace(with: (shallowCopy + term).flatten())
        }

    @discardableResult func flatten() -> Expr {
        args = args.flatMap{ $0.op == op ? $0.args : [$0] }
        return self
    }
}

func + (_ x: Expr, _ y: Expr) -> Expr { Expr("+",  x,  y) }

func test() {
    let expr = Expr("vector", "X", "1") + Expr("vector", "Y" + "2", "1")
    print(expr)
    expr.addVectors()
    print(expr)
}

test()

One last note: if we change flatten as below to explicitly set parent to nil when it creates a dangling node, then the crash goes away, which I read as evidence for y+2's weak parent reference to the temporary being the source of the crash.

    @discardableResult func flatten() -> Expr {
        args = args.flatMap{ expr -> [Expr] in
            if expr.op == op {
                expr.parent = nil
                return expr.args
                }
            else {
                return [expr]
            }
        }
        return self
    }

If I understand the implementation correctly: when the strong reference count goes to zero, the object deinit is run, but if the unowned ref count is non-zero, the memory associated with the object is left as an unowned husk until the unowned ref count goes to zero allow the memory to be freed.

Is the temporary's memory being freed while the y+2 node still has an unowned reference to it?
Might temporary have been allocated on the stack in add(onRight:)?

Here is a thread giving more context, though I think I wrote everything relevant here. https://twitter.com/RonAvitzur/status/1463576340519473159

4 Likes

I think there's at least a possibility that unowned(unsafe) here is actually a fix, and not just hiding the bug. If so, that's the first time I've ever seen that :joy:

Here's the logic: unowned still does reference counting, which means when the containing object deinits, it will go to try to decrement the unowned refcount in the dead object, which of course will crash. If there are genuinely no other uses besides that, unowned(unsafe) should avoid the reference counting entirely, which would mean the dead object is actually unused, which should be safe.

Obviously all bets are off if anything actually tries to use the object

1 Like

That doesn’t seem correct. The way unowned refcounting works is that an object with outstanding unowned references is destroyed but not deallocated. (Though if that isn’t happening that’s a bug.)

2 Likes

Ah, good point. Yeah, nevermind, this is odd.

2 Likes

:slight_smile:

I'm not skilled enough to single-step through the disassembly to tell what is actually going on. My best guess is that the temporary Expr created by (shallowCopy + term) is actually disposed of (and not just deinited) while the y+2 node strongly referenced by b[0] passed to add(onRight:) still has an unowned reference to it in parent. Attempting to decrement the temporary's unowned ref count when b[0] is gone would then be a crash.

But, as you say, that is not supposed to happen - the remaining unowned reference should ensure that the temporary's memory is left in place as a husk for just that purpose after it is deinited.

At least the test case is small now. When I started a few days ago, this file did a lot of work first before exhibiting the failure: FractalOctohedron

Trying both with an ASAN standard library/runtime might be interesting

1 Like

Using ASAN on my code here provided no new clues.

FWIW, the (first) code snippet runs on Playground 3.4.1 and print

+(vector(X,1),vector(+(Y,2),1))
vector(+(X,Y,2),+(1,1))
1 Like

AFAIK, Playground builds in debug mode and not in release mode.


It also crashes with a recent Swift 5.6 compiler (swiftlang-5.6.0.308.302).

I saw some calls to swift_bridgeObjectRelease around the crash so I though that maybe some bridging let it crash. And indeed, if you replace var args: [Expr] with var args: ContiguousArray<Expr> it does no longer crash:

final class Expr: CustomStringConvertible, ExpressibleByStringLiteral {
    unowned var parent: Expr? = nil
    var op: String
    var args: ContiguousArray<Expr> = [] { didSet { setArgParents() } }
    func setArgParents() { for arg in args { arg.parent = self } }
    subscript(index:Int) -> Expr { args[index] }
    var count: Int { args.count }

    init(_ op: String, _ args: ContiguousArray<Expr>) {
        self.op   = op
        self.args = args
        setArgParents()
    }
    convenience init(stringLiteral s: String) { self.init(s) }
    convenience init(_ op: String, _ args:Expr...) { self.init(op, ContiguousArray(args)) }

    var shallowCopy: Expr { Expr(op, args) }
    var description: String { args.isEmpty ? "\(op)" : "\(op)(\(args.map{$0.description}.joined(separator: ",")))" }

    func addVectors() {
        let a = args[0], b = args[1]
        args.remove(at: 1)
        for j in 0 ..< a.count {
            a[j].add(onRight: b[j]).flatten()
        }
        if count == 1 { replace(with: args[0]) }
    }

    @discardableResult func replace(with expr: Expr) -> Expr {
        op   = expr.op
        args = expr.args
        return self
        }

    @discardableResult func add(onRight term: Expr) -> Expr {
        replace(with: (shallowCopy + term).flatten())
        }

    @discardableResult func flatten() -> Expr {
        args = ContiguousArray(args.flatMap{ $0.op == op ? $0.args : [$0] })
        return self
    }
}

func + (_ x: Expr, _ y: Expr) -> Expr { Expr("+",  x,  y) }

func test() {
    let expr = Expr("vector", "X", "1") + Expr("vector", "Y" + "2", "1")
    print(expr)
    expr.addVectors()
    print(expr)
}

test()

This is not a sufficient evidence that some bridging is the root cause but it looks like it is at least some kind of bug and not the expected behaviour. Its probably worth filling a bug report at https://bugs.swift.org/

4 Likes

Alas, in my full application, I am using ContiguousArray<Expr> and seeing a crash. I had foolishly simplified that to [Expr] for the sake of brevity in the sample code. Using ContiguousArray in the sample doesn't crash here with my version of Swift either. I will need to go back to the original code and pare it down again to see how to replicate the problem I am seeing while using ContiguousArray.

4 Likes

That version with ContiguousArray works here if the project has Disable Safety Checks set to yes, but crashes with malloc: Non-aligned pointer 0x16fdff2d8 being freed when Disable Safety Checks is set to no.

Which makes sense if the claim that Disable Safety Checks makes unowned into unowned(unsafe) by default in memory management - What is the difference in Swift between 'unowned(safe)' and 'unowned(unsafe)'? - Stack Overflow is correct.

I'm compiling with Xcode 13.1, Apple Swift version 5.5.1 (swiftlang-1300.0.31.4 clang-1300.0.29.6).

1 Like

I used a new Swift Package with default settings but compiled in release mode. AFAIK, Safety Checks should be enabled. With those settings I do not experience the crash with ContiguousArray but with Array.

Regardless of that, the code also does only ever set the parent to self in setArgParents() and never actually uses it in any way. self is guaranteed to be valid so we should write valid data into parent. The code does not even read the unowned variable. Because after the parent is set only the swift runtime does anything with parent, it looks like a compiler/runtime bug to me.

1 Like

I used a new command-line tool for my project here.

Using the latest development snapshot toolchain Apple Swift version 5.6-dev (LLVM 27222cde774bd6b, Swift 17f1cf92973d2c1) I see the crash regardless of Disable Safety Checks settings.

If so, ooh - my first compiler bug, cool! That's a time-honored right of passage learning a new language. It puts me out of my depths, trying to diagnose it, though.

3 Likes

Feedback submitted: FB9782558. Is there a preferred place for submitting suspected runtime bugs?

1 Like

Compiler bugs can be reported at bugs.swift.org

2 Likes

Thank you. Reported at: [SR-15527] Crash with unused unowned reference to deinited object - Swift

2 Likes

Fixed in EscapeAnalysis: fix a bug which results in not detecting an escape through an unowned reference. by eeckstein · Pull Request #40325 · apple/swift · GitHub

Papa, today I am a real Swift programmer - I found a compiler bug! :)

4 Likes
Terms of Service

Privacy Policy

Cookie Policy