Enhancement request: optionally allow @WeakOuter to be the default (with new @StrongOuter annotation)

[yegork78]

The request is for a new command line argument for j2objc to optionally consider all outer references "weak". In this mode all outer references are week and a new annotation @StrongOuter can be used to make a specific inner class reference outer object strongly.

[tomball]

The preferred pattern (Effective Java #22) is to mark all inner classes as static unless there is an actual need for the inner class to reference the outer class. By marking an inner class static, the compiler (javac or j2objc) won't define an outer instance field, reducing the coupling of the two classes and making the inner class more efficient. Your app should need very few inner classes that are not static.

For what it's worth, the only reason for needing @WeakOuter is for those few classes which return inner class instances that may outlive the outer instance. For example, HashMap has inner class collections of keys, values, and entries, and accessors that return those instances. In the whole JRE, there are only 16 classes that use this pattern, and they all need a WeakOuter. If your app is using this pattern widely, consider having those accessors return copies instead.

[yegork78]

I do know the difference between regular inner classes and static ones. :-)
I was only talking about proper inner classes.

We are constructing GUI in J2ObjC, so we have a lot of these.

1. Form Appointment holds a button "Save", which has an OnClick event, which is a lambda expression within the Appointment class.
   So, this is a strong reference loop: Form -> Button -> Lambda -> Form.
   If we make the Button -> Lambda reference weak, all lambda will be released immediately.
   If we make the Form -> Button reference weak, the button will be released immediately.
   So it only makes sense to make the Lambda -> Form reference weak (which is actually an Outer reference, since lambdas are treated simply as inner classes by J2ObjC), especially taking into account that the OnClick event will never exist beyond the scope of the form.
   We have about 100 lambdas like that (all are OnClick and OnChange events).

2. Consider an appointment form, constructed to present an appointment.
   Within the form we have several charges listed one after another.
   The form has a strong reference to all its components (components have weak reference to their parent).
   But the charges visual components always live in the context of the appointment form and are defined as inner classes, using the functions of the outer class. While making them static and going through the "parent" reference whenever they need to perform a call on the parent is possible, it's cumbersome and creates too strong of a coupling on the GUI design.
   We have about 20 inner classes like that.

My additional problem here was that I don't actually want my developers, developing the shared code to remember inserting "@weak" everywhere, cause they won't. Also consider the difficulty of declaring a Lambda as a @weak reference (define a separate variable, etc.).
We have about 120 outer references, out of which we only found 2 references, where outer should be strong.

[danieldickison]

It seems like this boils down to whether the default is "safe but possibly leaky" vs "unsafe but less likely to leak". I think j2objc's current default is a good choice, since a pass through the Leaks instrument by someone familiar with ObjC memory semantics is critical for finding leaks regardless of this issue.

To me it seems like being able to mark anonymous classes and lambdas as @WeakOuter is key in making this more manageable. This would be analogous to writing [unowned self] in Swift blocks that are used as UI element callbacks, which is a habit that is entirely possible for UI devs to get into. While it would require developers to remember to use the annotation, at least it wouldn't force writing verbose named inner classes wherever you just need a single callback closure. I understand there's a roadblock to doing so due to Java 7 compatibility (https://j2objc.blogspot.com/2017/01/breaking-retain-cycles-with-weak-and.html), and I'm not sure if this is something that is being worked on (#105 is relevant but there's no info there).

[tomball]

What would break if a lambda's outer reference was always __weak? Any thoughts on making Java 8 the minimum OS?

[yegork78]

If I may just offer an opinion on "safe but possibly leaky" vs "unsafe but less likely to leak".
I do love type safety, strong types and everything else Java had to offer. That was the reason I chose Java+GWT+J2ObjC, instead of some JS approach or something, but:

I have 5 developers developing that client application and 2 QAs testing it. None of those developers ever developed anything in ObjC or worked with a Mac!
Those 5 developers simply won't remember to insert the annotations. The main reason is – it will work without the annotations.

Once I changed J2ObjC source to make outers weak by default, I asked one of the QAs to run the app in the debugger and "do the app flow". And he found the needed two strong outers in less than half an hour!

Fixing a memory leak is very hard: as you said, you need a serious amount of time, by a good developer, familiar with Apple tools.
On the other hand, finding a bug with having an object released too early is very simple: just look at the failure in the debugger and notice which variable was referenced, while being null, that's it! (obviously, if @weak actually generates __weak, not __unsafe_unretained).

I'm obviously not saying that Weak should be the default out of the box, but having such an option in a centralised manner, I would say, could be a great option.

Thank you!

[sarsonj]

I agree, that it would be nice to have this as j2objc translator option. We are in process of cleaning memory leaks, our code is using tons of callbacks and it is really nightmare.