Uploaded image for project: 'Swift'
  1. Swift
  2. SR-4756

If NSCopyObject is called on a Swift class with stored properties, it can cause a crash

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Open
    • Priority: Medium
    • Resolution: Unresolved
    • Component/s: None
    • Labels:
    • Environment:

      macOS 16E195
      Xcode 8E2002

      Description

      When an NSTextFieldCell subclass contains Objective-C-compatible reference-type properties, and is loaded from a .nib file in which its containing text field has its Baseline aligned to some other object via autolayout, its properties get over-released when the cell is deallocated. A simple example that will cause the crash is:

      @objc(Foo) class Foo: NSObject {}
      
      @objc(CustomTextFieldCell) class CustomTextFieldCell: NSTextFieldCell {
          let foo = Foo()
      }
      

      The equivalent code in Objective-C works properly and does not crash:

      Unable to find source-code formatter for language: objective-c. Available languages are: actionscript, ada, applescript, bash, c, c#, c++, cpp, css, erlang, go, groovy, haskell, html, java, javascript, js, json, lua, none, nyan, objc, perl, php, python, r, rainbow, ruby, scala, sh, sql, swift, visualbasic, xml, yaml
      #import <Cocoa/Cocoa.h>
      
      @interface Foo: NSObject
      @end
      
      @implementation Foo
      @end
      
      @interface CustomTextFieldCell: NSTextFieldCell
      
      @property (nonatomic, strong) Foo *foo;
      
      @end
      
      @implementation CustomTextFieldCell
      
      - (instancetype)initWithCoder:(NSCoder *)coder {
          self->_foo = [Foo new];
          
          return [super initWithCoder:coder];
      }
      
      @end
      

      The problem seems to occur, as far as I can tell, because when the Baseline layout relation is applied, AppKit copies the text field’s cell. Subsequently, NSCell’s -copyWithZone: method calls NSCopyObject, which in turn calls a private function named “fixUpCopiedIvars.” With an Objective-C cell class, fixUpCopiedIvars calls class_getIvarLayout, and retains all its instance variables, so both the original cell and the copy have an owning reference to all of them. This retain is then balanced by a release when the cell is deallocated. With a Swift cell class, however, class_getIvarLayout returns NULL, so the ivars are never retained; however, this nonexistent retain is still balanced by a release when the cell is deallocated. The result is that the program accesses freed memory, leading to a crash or worse.

      I have attached a sample project which demonstrates the problem.

      Steps to Reproduce:
      1. Compile and run the attached project with zombies enabled.

      2. Open a new document, and then close it.

      3. Notice that you crash when the program tries to over-release the zombie object.

      4. Now disable CustomTextFieldCell.swift, enable CustomTextFieldCell.m, and notice that everything now works.

      Expected Results:
      Even though NSCopyObject is old and deprecated, it may be called on Swift subclasses of Objective-C by legacy code, including code in Apple's own frameworks. Therefore, it should probably interact with it in ways that don't cause crashes.

      Actual Results:
      If NSCopyObject is called on a Swift subclass of an Objective-C class which has an Objective-C-compatible stored property, the property in the copy is over-released, leading to a crash.

      Note:
      Most workarounds don't work.

      Implementing one's own copy(with:) method fails, because the crash occurs when calling super's implementation.

      Using a weak variable to the containing NSControl doesn't work, because NSCopyObject doesn't play nice with weak variables either, causing errors to be logged to the console. Using unowned doesn't work, because the property cannot be null, and the connection will not be made until runtime.

      Storing the property using objc_setAssociatedObject does work, but this is a cumbersome solution.

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              CharlesJS Charles Srstka
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated: