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

[Anthony Petrov]

Thanks, Petr. That sounds good. I'm fine with the new fix.

--
best regards,
Anthony

On 3/14/2014 11:35 AM, Petr Pchelko wrote:
> 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.
>>>>
>>>>
>>
>