Owned instance-method returns that hit a cached wrapper leak a retain (immutable copy, alloc().init() with a JS initializer) #566

Summary

When an instance method returns an object it owns, and that object already has a JS wrapper, Interop::GetResult returns the cached wrapper without releasing the returned +1. The object leaks one retain per call. The uncached path does release it.

This covers two groups of methods:

• the copy, mutableCopy and new families, and ns_returns_retained methods;
• initializers called as X.alloc().init…().

Owned returns

The common case is copy on an immutable object, which returns [self retain]:

const array = NSArray.arrayWithArray([1, 2, 3]);
const before = array.retainCount();
for (let i = 0; i < 50; i++) array.copy();
// after a GC and an autorelease drain: array.retainCount() === before + 50

Initializers

X.alloc().init…() allocates the object (+1) and calls the initializer on it. If the object gets a JS wrapper during that call, the initializer's result hits the cache and the alloc +1 is never released. That happens whenever the initializer is implemented in JS, or calls any JS-implemented method on the object:

const Cell = UITableViewCell.extend({
  initWithStyleReuseIdentifier(style, id) {
    return UITableViewCell.prototype.initWithStyleReuseIdentifier.call(this, style, id);
  },
});
const cell = Cell.alloc().initWithStyleReuseIdentifier(0, "cell");
// the retain count stays at 2 or more once JS drops `cell`, so it is never freed

new X() already releases this +1 (ArgConverter::ConstructObject), and #564 fixes X.new() and objects created by native code. alloc().init…() is the remaining construction path.

Effect on the wrapper lifetime work

With #564, and under the wrapper lifetime modes (#559 and up), the extra retain reads as a native owner, so the wrapper stays a strong root and the object is kept forever.

#564 releases the +1 on a cache hit for class methods and C functions only (the case it needed was +new on a class with a JS initializer), and leaves instance methods as they are.

Why instance methods can't simply release

Two kinds of owned instance returns hand back an object without a net +1. Releasing on a cache hit over-releases both (a regression test in #564 crashes without the restriction):

1. NS_REPLACES_RECEIVER / ns_consumes_self methods, e.g. -awakeAfterUsingCoder:. They return their receiver, consuming the reference the caller passed in. The metadata generator flags them as owning their return (ns_returns_retained) but records nothing for ns_consumes_self.
2. JS-implemented owned-return methods reached through native dispatch, e.g. a JS copyWithZone: behind native -copy. ArgConverter::SetValue hands the JS result back to native code at +0, while the caller expects +1. This is a bug in its own right: native callers of such a JS method get an object they don't own and may over-release it.

The same +0 return reaches class methods through a JS initializer

Kind 2 is not limited to instance methods. +new, or any native factory that ends in [[self alloc] init…], returns whatever the initializer returns. If a JS-implemented initializer returns an object other than its receiver, that object arrives at +0:

• the initializer returns some other wrapped object on purpose;
• super.init…() substituted the receiver (a class cluster, a cached instance), and the JS initializer returns that result. The inner call already released the substitute's +1 when it wrapped it.

A caller that then releases on a cache hit consumes the reference Instances holds, and the object is freed while its wrapper is live. new X() does this today, and the class-method release in #564 does the same for X.new(). A JS initializer that returns its receiver is fine: the alloc +1 is still there.

Suggested fix

1. In ArgConverter::MethodCallback, retain the result of a JS-implemented method whose selector returns an owned object: the copy/mutableCopy/new families or ns_returns_retained, excluding the init family and consumes-self methods. That fixes native callers (2) and gives every owned return a real +1.
2. Record ns_consumes_self in the metadata, or keep a small list of the known NS_REPLACES_RECEIVER selectors.
3. Then GetResult can release on every owned cache hit except consumes-self methods.
4. Initializers need their own rule, because they are consumes-self and step 3 skips them:
   • alloc().init…(): ArgConverter::Invoke knows when it allocated the target. On a cache hit where the result is that target, the surplus reference is provably the alloc +1, so releasing it is safe whether the initializer is native or JS. This part doesn't depend on steps 1 to 3.
   • A JS initializer that returns another object: it has to reach its native caller at +1, like the owned returns in step 1. What to do with the receiver's reference depends on whether a super initializer already consumed it, so this needs more care than a plain retain.

Because it changes the ownership contract of JS-implemented methods towards native callers, this fits a minor release better than a patch. The alloc().init…() release in step 4 is the exception: it only drops a reference the runtime itself created.

Tests to add

• immutable.copy() keeps the retain count unchanged (it was a spec in the first revision of fix(runtime): release wrappers made for JS methods called from native code #564).
• -awakeAfterUsingCoder: returning its receiver doesn't free it.
• A JS copyWithZone: called through native -copy: the native caller owns the result (for example, [[obj copy] release] from a fixture doesn't free an object JS still uses), and calling it from JS doesn't over-release.
• X.alloc().init() on a class with a JS initializer: the object is freed once JS drops it, and exactly once.
• X.new() and new X() on a class whose JS initializer returns a different wrapped object: that object survives, checked under AddressSanitizer.

The two initializer cases were found by reading the code paths and have no reproduction yet; the last two specs would be the first.