Xcode 10 crashes with ReadWriteSignal

[egarni]

Expected Behavior

The ReadWriteSignal setter should not crash

Current Behavior

ReadWriteSignal crashes when trying to access its setter

Steps to Reproduce

Here is a simple unit test to reproduce the crash

    func testReadWriteSignalCrashing() {
        ReadWriteSignal(CGPoint.zero).value.x = 2 // This one crashes for some reason
    }

Note that these tests are passing:

    func testReadWriteSignalNotCrashing() {
        ReadWriteSignal(CGPoint.zero).value = CGPoint(x: 2, y: 0) // This is fine
        let signal = ReadWriteSignal(CGPoint.zero)
        signal.value.x = 2 // This is also fine
    }

Context

The test fails when building with Xcode 10

Failure Logs

Error:
Thread 1: Fatal error: Unexpectedly found nil while unwrapping an Optional value

[egarni]

@mansbernhardt We are experiencing strange crashes with Flow and Xcode 10.
Really hard to understand what's going on. Maybe you have some ideas about it?

[mansbernhardt]

@fennecOS I have tried different work-arounds such as storing the setter in the CoreSignal instead of using an associated object. But it will still crash. This must be a regression in the compiler. I would see if we could reduce this to a smaller sample and file a radar.

[mattiasjahnke]

Adding a deinit to CoreSignal shows that the signal instance seems to be deallocated before we hit the set on value in ReadWriteSignal

[egarni]

Yes, I suspected the associated object. But it’s not that.
I was also ordering about the setter being an optional parameter in the init, and therefore cannot explicitly be marked as escaping, but this does not seems to be the case either.

[mansbernhardt]

@mattiasjahnke This is a really interesting finding. This really seems to indicate that the compiler (and ARC) has some serious bug in it?

[mansbernhardt]

So by adding a retain hack in CoreSignal's init (let _ = Unmanaged.passRetained(self)), we make the test pass. But of course we will leak and several other tests will fail.

Really strange that the instance is deallocated to early. Not sure how we can work around this? I would suggest filing a bug with Apple and point to our GitHub repo and provide the unit test above?

[mansbernhardt]

I tested to compile with the snapshots for 4.2 and 5.0 at https://swift.org/download/#snapshots. Both compiler versions crashes (Abort trap: 6) for the line:

ReadWriteSignal(CGPoint.zero).value.x = 2

So this line does not even compile on those versions.

[mansbernhardt]

Is the above use-case something that comes up in your code base?

[egarni]

yes it's used in several places. We have some computed properties returning a ReadWriteSignal and we sometimes use it just to set the value.

[egarni]

I played a bit with this bug, and this is the minimum of code I could find that reproduces the issue:

protocol SomeProtocol { }
class SomeClass: SomeProtocol { }

extension SomeProtocol {
    var someGetter: CGPoint {
        nonmutating set { _ = self }
        get { return .zero }
    }
}

func testCrashingSetter() {
    SomeClass().someGetter.x = 2
}

I suspect that it has something to do with the nonmutating key-word. I guess the compiler is a bit too agressive since I guess it assumes that the object is safe to be deallocated because it won't be mutated.

[jckarter]

@fennecOS If your example crashes, then it's definitely a bug. It's true that when the setter is nonmutating, then there is nothing formally keeping SomeClass() alive after someGetter has been accessed, but that's because nothing should even be referring back to the object instance. Deallocating it early should not cause the program to crash. However, I tried copying your program to a single swift file, compiling it, and was unable to reproduce the crash:

$ cat ~/foo.swift
import CoreGraphics

protocol SomeProtocol { }
class SomeClass: SomeProtocol { }

extension SomeProtocol {
    var someGetter: CGPoint {
        nonmutating set { _ = self }
        get { return .zero }
    }
}

func testCrashingSetter() {
    SomeClass().someGetter.x = 2
}

$ xcrun swiftc -wmo -O ~/foo.swift
$ ./foo

Are there any other settings in your build environment that I should try to reproduce? Do you have a complete Xcode project we can look at? If you haven't yet, and you have a moment, we'd appreciate it if you could file a bug at bugs.swift.org or bugreport.apple.com.

[mansbernhardt]

@jckarter Thanks for having a look at this.

One problem with your example above is that this code never calls testCrashingSetter().

Another problem seems to be that running it from the terminal does not trigger memory issues. Perhaps because it is not run through a debugger? But when you run it from Xcode you get a SIGABRT and the log says:

crash(71664,0x1007e35c0) malloc: *** error for object 0x100d0a920: pointer being freed was not allocated
crash(71664,0x1007e35c0) malloc: *** set a breakpoint in malloc_error_break to debug

I have attached a Xcode project with mac command line app that you can run instead:

crash.zip

The work around we came up with was by modifying the example above to add the property to SomeClass as well:

class SomeClass: SomeProtocol {
    var someGetter: CGPoint = .zero
}

[jckarter]

Sorry, that was a paste-o. In my real test, I had SomeClass().someGetter.x = 2 at top level. Thanks for the complete reproducer!

[jckarter]

I filed https://bugs.swift.org/browse/SR-8990. If you find out any more information, we'd appreciate it if you could add it to the bug report too. Thanks again.

[jckarter]

@mansbernhardt Were you able to reproduce the assertion failure you saw in #34 (comment) with that project too? I tried the latest snapshots for both 4.2 and trunk and did not see a compiler assertion failure, though the runtime crash occurs in all of them.

[mansbernhardt]

@jckarter I could not reproduce the crash with the sample I provided above (crash.zip) on the toolchains that crashed on the original Flow code. But in the updated sample below that is closer to the original Flow code and could get it crash:

crash 2.zip

import CoreGraphics

protocol SomeProtocol {
    associatedtype Value
    var inner: SomeClass<Value> { get }
}

class SomeClass<Value>: SomeProtocol {
    var inner: SomeClass<Value> { return self }
    var value: Value
    init(_ value: Value) {
        self.value = value
    }
}

extension SomeProtocol {
    var someGetter: Value {
        nonmutating set { inner.value = newValue }
        get { return inner.value }
    }
}

SomeClass<CGPoint>(.zero).someGetter.x = 2

[jckarter]

Thanks!

[jckarter]

I think I have a fix: apple/swift#19902 I asked our CI system to build a toolchain, which you'll hopefully be able to try to see if it addresses the problem for you.