<AWT Dev> [9] Review Request: 8037099 [macosx] Remove all references to GC from native OBJ-C code

[Petr Pchelko]

Hello, Anthony.

I've reverted the CFRetainedResource back. Using CFRelease there is better because it would work in case the resource is a CoreFoundation object, not Cocoa object. As for your idea with different memory management - I could not find any proof, but I could not prove that it's wrong either. So let's better leave this piece as is.

I did not make a new webrev, it's the same, just CFRetainedResource changes reverted.

With best regards. Petr.

On 14.03.2014, at 0:42, Petr Pchelko <petr.pchelko at oracle.com> wrote:

> Hello, Anthony.
> 
>> A comment regarding changes in src/macosx/native/sun/awt/CFRetainedResource.m: I'm not really an expert in Obj-C memory management, but IIUC, the autorelease pool created with the JNF_COCOA_ENTER will try and retain the obj reference when passing it to the block that is dispatched to the AppKit thread. It will subsequently call -release too. The block is dispatched asynchronously, and it could happen that the AppKit thread processes the request before we call the -release in JNF_COCOA_EXIT. This may result in -release being finally called off of the AppKit thread - opposite to what the nativeCFRelease(..., true) method call was intended for.
> Hm… Interesting idea, I’ve had a hard time trying to understand what’s the reason for this strange arrangement. Let me investigate that. 
> 
>> In src/macosx/native/sun/awt/LWCToolkit.m:
>>> 296 JNF_COCOA_ENTER(env);
>>> 297     // We double retain because this object is owned by both main thread and "other" thread
>>> 298     // We release in both doAWTRunLoop and stopAWTRunLoop
>>> 299     result = ptr_to_jlong([[[AWTRunLoopObject alloc] init] retain]);
>>> 300 JNF_COCOA_EXIT(env);
>> 
>> I believe we need to call -retain twice, otherwise one of the -release calls in either doAWTRunLoopImpl or stopAWTRunLoop will fail since the object will have already been released.
> No, this is actually fine, because the object after alloc already has the retainCount = 1. So after this code the object will have retainCount = 2 and it’s exactly what we want to achieve.
> 
> With best regards. Petr.
> 
> 14 марта 2014 г., в 12:29 до полудня, Anthony Petrov <anthony.petrov at oracle.com> написал(а):
> 
>> Hi Petr,
>> 
>> A comment regarding changes in src/macosx/native/sun/awt/CFRetainedResource.m: I'm not really an expert in Obj-C memory management, but IIUC, the autorelease pool created with the JNF_COCOA_ENTER will try and retain the obj reference when passing it to the block that is dispatched to the AppKit thread. It will subsequently call -release too. The block is dispatched asynchronously, and it could happen that the AppKit thread processes the request before we call the -release in JNF_COCOA_EXIT. This may result in -release being finally called off of the AppKit thread - opposite to what the nativeCFRelease(..., true) method call was intended for.
>> 
>> So I think the strange arrangement of the COCOA_ENTER/EXIT block in that method should persist. Perhaps it needs a comment explaining why we do this.
>> 
>> 
>> In src/macosx/native/sun/awt/LWCToolkit.m:
>>> 296 JNF_COCOA_ENTER(env);
>>> 297     // We double retain because this object is owned by both main thread and "other" thread
>>> 298     // We release in both doAWTRunLoop and stopAWTRunLoop
>>> 299     result = ptr_to_jlong([[[AWTRunLoopObject alloc] init] retain]);
>>> 300 JNF_COCOA_EXIT(env);
>> 
>> I believe we need to call -retain twice, otherwise one of the -release calls in either doAWTRunLoopImpl or stopAWTRunLoop will fail since the object will have already been released.
>> 
>> --
>> best regards,
>> Anthony
>> 
>> On 3/13/2014 7:45 PM, Petr Pchelko wrote:
>>> Hello, AWT Team.
>>> 
>>> Please review a huge but simple cleanup fix.
>>> 
>>> The bug: https://bugs.openjdk.java.net/browse/JDK-8037099
>>> The fix: http://cr.openjdk.java.net/~pchelko/9/8037099/webrev.01
>>> 
>>> Now the Objective-C Garbage Collector is completely deprecated and we do not use it and will never use. But we still have some code that was used for GC.
>>> The problem is that under GC retain/release is not the same as CFRetain/CFRelease, but now it's absolutely the same.
>>> 
>>> I've replaced all CFRetain/CFRelease to retain/release where possible, deleted the pattern CFRetain(o); [o release]; and removed finalize overrides.
>>> I know that in some places retain is not needed. But in this fix I've left it as is, because it's only a preparation for a big-native-memory-management-fix I'm preparing.
>>> 
>>> Thank you.
>>> With best regards. Petr.
>>> 
>>> 
>