[SR-1042] Make "lazy var" threadsafe

[drewcrawford]

Previous ID	SR-1042
Radar	None
Original Reporter	@drewcrawford
Type	Bug

Environment

swift-DEVELOPMENT-SNAPSHOT-2016-03-16-a

Additional Detail from JIRA

Votes	8
Component/s	Compiler
Labels	Bug, LanguageFeatureRequest, Runtime
Assignee	None
Priority	Medium

md5: 3e0b2bd438a588ec34ca8ff1b3ddeff2

is duplicated by:

• SR-7712 Sync dispatch queue block crash

Issue Description:

In the Swift book it says

If a property marked with the lazy modifier is accessed by multiple threads simultaneously and the property has not yet been initialized, there is no guarantee that the property will be initialized only once.

Meanwhile, libdispatch maintainers have decided to make dispatch_once unavailable to Swift and advising to use lazy modifier as a replacement.

As a result, there is currently no safe way to achieve "run exactly once" behavior in Swift.

We need some way to do this. I propose the easiest path is to make "lazy var" threadsafe, but if not, there should be some API (stdlib, libdispatch, I don't care) to achieve the "run exactly once" behavior.

I am tentatively marking this "compiler", but feel free to move it around.

[drewcrawford]

cc: dgrove-oss (JIRA User) @parkera

[belkadan]

static let is a thread-safe single-initialization mechanism. I think it's not possible to make a thread-safe single-initialization mechanism for a property without external synchronization, but I suppose there could be a side table like the old @swift-cihronized(self).

(IIRC dispatch_once was never safe for instance properties in Objective-C either. See this StackOverflow answer by @gparker42.)

[belkadan]

I think you'll also get pushback from people who want performance, but maybe we'll have lazy and lazy(singlethreaded) or something. Needs design.

[jckarter]

This seems like something property behaviors could eventually address as a library feature too.

[gparker42]

A thread-safe lazy property would require a memory barrier on every read on some architectures. (`dispatch_once` avoids that barrier, but it is unsafe for anything other than global variables.)

A thread-safe lazy property that is too large or does not have some unused sentinel value available would also need additional storage for synchronization (stored next to property value, or in the object header, or in a side table like `@swift-cihronized`).

Both of these are expensive and may not be suitable for every lazy property.

[ddunbar]

+1 to this idea. Not only is there no guarantee that the value will be initialized only once, concurrent access to a lazy variable may cause crashes in otherwise safe code.

My current view is that using "lazy" is almost an anti-pattern for writing complicated concurrent Swift code because it is so easy for code to "look" safe.

The other reason it would be nice to address this in the compiler is that it is hard to come up with a convenient library method for this behavior (I at least haven't been able to get one). What I currently use for this is something like:
swiftlang/swift-package-manager@a086f6c
but that implementation is both cumbersome to use and rather inefficient.

[ddunbar]

This continues to be a regular source of crashes when people forget and use `lazy` in otherwise safe looking code.

This does not seem like the right default for a "safe by default" language.

[belkadan]

I'm not sure if this needs the full Swift Evolution Process, but discussion should certainly happen on swift-evolution, not here.

[swift-ci]

Comment by Francisco (JIRA)

Hello! I will share my experience with a nasty bug that happened today to a bunch of users of our app (Mimo).
Basically, I had something in pseudocode like this:

class MyViewController: UIViewController {    

    lazy var _view: MyView = {
        let myView = MyView()
        self.view.addSubview(myView)
        return myView
    }()

    func cellForRowAtIndexPath(collectionView: UICollectionView) {
        if collectionView = _view.firstCollectionView {        
            ...
        } else {
            collectionView.dequeCell(MyCellIdentifier) 
            ...
        }
    }
}

class MyView: UIView {

    let firstCollectionView = UICollectionView()
    let secondCollectionView = UICollectionView()

    init() {
        secondCollectionView.registerCell(MyCellIdentifier)
    }

}

The app crashed with the error could not dequeue a view of kind: UICollectionElementKindCell with identifier MyCellIdentifier - must register a nib or a class for the identifier

After debugging for a while and trying to understand what happened, I checked the addresses of the collection views by putting a breakpoint in cellForRowAtIndexPath:

collectionview(of cellForRowAtIndexPath): 0x00007f9ae0873200
firstCollectionView = 0x00007f9ae3194200
secondCollectionView = 0x00007f9ae017ac00

To my suprise, the three addresses were different, and I expected to see the collection view address to the same as secondCollectionView's one.

After that, I tried adding an identifier to both the first collection view and the second collection view. After doing that and checking the identifier, I found that the collectionview which was dequeing the cell (parameter of cellForRowAtIndexPath) was firstCollectionView instead of secondCollectionView.

After all of this, I then found out that the lazy UIView initialization closure was being called two times. This was a difficult bug to spot, specially because the crash was on the cell dequeuing.

So what I don't know is why the reference of the collectionview received at cellForRowAtIndexPath was pointing at firstCollectionView (I don't believe it's just coincidence), but still it isn't the point of this post. I'm just trying to show an unexpected bug with lazy var not being threadsafe. I know it's my fault for not thinking about this while implementing it, but I thought it would be relevant to share a practical bug related with not having thread safe lazy vars.

Thanks for reading!