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 <[email protected]> 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 > <[email protected]> написал(а): > >> 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. >>> >>> >
