swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.61k stars 10.37k forks source link

[SR-15012] stored properties should be destroyed in reverse order #57341

Open swift-ci opened 3 years ago

swift-ci commented 3 years ago
Previous ID SR-15012
Radar rdar://problem/81466983
Original Reporter nivekresearch (JIRA User)
Type Bug
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: f09575bbab5ddcfca12ffeb74f158d1a

Issue Description:

C++, Objective-C and other language initialize instance variables, member variables or properties in the order declared and the destroy them in the reverse order; however, it Swift initializes and destroys in the order declared. The can result in errors introduced when code is converted from Objective-C to Swift simply because stored properties are destroyed in a different order. A simple example, can be found at https://developer.apple.com/documentation/swift/cocoa_design_patterns/using_key-value_observing_in_swift where `observation` needs to be destroyed before `objectToObserve` to be certain that `MyObserver` can never cause a crash.

Sample code "ObjectiveCObject.h":

#import <Foundation/Foundation.h>

NS_ASSUME_NONNULL_BEGIN

@interface ObjectiveCObject: NSObject

- (instancetype) initWithName:(NSString*) name;

@end

@interface ObjectiveCObjectWrapper: NSObject

@property ObjectiveCObject* a;
@property ObjectiveCObject* b;
@property ObjectiveCObject* c;
@property ObjectiveCObject* d;
@property ObjectiveCObject* e;

@end

NS_ASSUME_NONNULL_END

Sample Code "ObjectiveCObject.m":

@interface ObjectiveCObject ()

@property (copy) NSString* name;

@end

@implementation ObjectiveCObject

- (instancetype) initWithName:(NSString*) name {
    if ((self = [super init]) != nil) {
        self.name = name;
        NSLog(@"init %@", self.name);
    }
    return self;
}

- (void) dealloc {
    NSLog(@"dealloc %@", self.name);
}

@end

@implementation ObjectiveCObjectWrapper

- (instancetype) init {
    if ((self = [super init]) != nil) {
        _a = [[ObjectiveCObject alloc] initWithName:@"A"];
        _b = [[ObjectiveCObject alloc] initWithName:@"B"];
        _c = [[ObjectiveCObject alloc] initWithName:@"C"];
        _d = [[ObjectiveCObject alloc] initWithName:@"D"];
        _e = [[ObjectiveCObject alloc] initWithName:@"E"];
    }
    return self;
}

@end

Sample Code "SwiftObject.swift":

class SwiftObject: NSObject {

    private var name: String

    init(name: String) {
        self.name = name
        super.init()
        print("init \(name)")
    }

    deinit {
        print("deinit \(name)")
    }

}

class SwiftObjectWrapper: NSObject {

    private var a = SwiftObject(name: "A")
    private var b = SwiftObject(name: "B")
    private var c = SwiftObject(name: "C")
    private var d = SwiftObject(name: "D")
    private var e = SwiftObject(name: "E")

}
@main
class AppDelegate: NSObject, NSApplicationDelegate {

    func applicationDidFinishLaunching(_ aNotification: Notification) {
        let _ = SwiftObjectWrapper()
        let _ = ObjectiveCObjectWrapper()
    }

}

Results in the following output:

init A
init B
init C
init D
init E
deinit A
deinit B
deinit C
deinit D
deinit E
2021-08-02 20:19:11.918928-0400 StaticMap[18321:6349897] init A
2021-08-02 20:19:11.918980-0400 StaticMap[18321:6349897] init B
2021-08-02 20:19:11.919010-0400 StaticMap[18321:6349897] init C
2021-08-02 20:19:11.919037-0400 StaticMap[18321:6349897] init D
2021-08-02 20:19:11.919064-0400 StaticMap[18321:6349897] init E
2021-08-02 20:19:11.919090-0400 StaticMap[18321:6349897] dealloc E
2021-08-02 20:19:11.919113-0400 StaticMap[18321:6349897] dealloc D
2021-08-02 20:19:11.919137-0400 StaticMap[18321:6349897] dealloc C
2021-08-02 20:19:11.919162-0400 StaticMap[18321:6349897] dealloc B
2021-08-02 20:19:11.919183-0400 StaticMap[18321:6349897] dealloc A

Note Swift is A, B, C, D, E for both init and deinit while Objective-C is A, B, C, D, E on init and then E, D, C, B, A on deallocate

typesanitizer commented 3 years ago

@swift-ci create

jckarter commented 3 years ago

Swift doesn't intentionally guarantee any particular order at all for the stored properties to get deinitialized in. I would strongly recommend making it so that you make it so that your deinits do not rely on happening in any particular order, because that order could be easily disturbed anyway by other references to the object in the system. If you nonetheless must make the properties get cleared in a specific order, you could perhaps reassign them all to `nil` in the required order explicitly in your `deinit`, so that changes to the compiler or runtime implementation in the future don't disturb the order.

swift-ci commented 3 years ago

Comment by Kevin (JIRA)

My concern is that the initialization IS in a particular order and the de-initialization IS in the same order not the reverse order as in other languages. And more particularly code that behaved properly under Objective-C does not behave properly in Swift for the simple reason that de-initialization happens different then in Objective-C.

And FWIW assigning `nil` to the stored properties above isn't allowed as the properties are not optional.

I doubt seriously there will be a satisfactory resolution to this issue and more than likely any change will burn someone. However, it would have been extremely helpful to have known that Swift did not behave like Objective-C when it came to destroying stored properties. And I can only hope that someone on the Swift team is willing to help others avoid the pitfalls I've had to endure by making the deinit behavior either public knowledge or consistent with Objective-C.

Lastly here's a more real example where the deinit order causes real issues that are not obvious because Swift doesn't behave like other languages (Objective-C):

@objc
class SwiftObj : NSObject {
    @objc dynamic
    var progress: String = "A"
}

@objc
class SwiftObserver: NSObject {

    @objc dynamic
    var swiftObj = SwiftObj()

    private lazy var observation: NSKeyValueObservation = {
        return swiftObj.observe(\.progress) { swiftObj, _ in
            print(swiftObj.progress)
        }
    }()

}

Here `swiftObj` is released first and then the observation which now cannot cleanup because the weak reference hidden in the `NSKeyValueObservation` (technical the private helper object) cannot be resolved. And because of the nature of KVO this can reek a lot of havoc.