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 <[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.
