This looks reasonable to me. Did you test the fix using the "leaks" to determine that the descriptors or the block itself were not over-retained?
Regards, Mike Swingler Apple Inc. On May 27, 2013, at 6:53 AM, Anthony Petrov <[email protected]> wrote: > Thanks for testing, James. I'm fine with the fix then. > > Note that we need at least one more review from a reviewer on this mailing > list before we can push your fix to the repository. > > Anyone? > > -- > best regards, > Anthony > > On 05/24/13 22:39, James Tomson wrote: >> Hi Anthony, >> >> Thanks for taking a look. To answer your questions, the >> application:openFiles and application:printFiles methods should not need >> similar special treatment - the passed instances should be getting >> implicitly retained and released by the block from my understanding, and >> can be queued for later processing without a problem. Testing locally >> with those handlers, the OpenFilesEvent and PrintFilesEvent events on >> the java-side are delivered without issue when the app is launched from >> an open file or print file event from the Finder. >> >> The issue with the passed NSAppleEventDescriptors is that while the >> objects are properly retained and released by the block, the Apple Event >> Handling system seems to be marking the instance's internal state as >> otherwise invalid/expired even though the instance itself is still >> retained. I'm unable to find any specific documentation or discussion >> about the lifecycle of these event descriptor objects though. >> >> -James >> >> >> On Fri, May 24, 2013 at 12:13 PM, Anthony Petrov >> <[email protected] <mailto:[email protected]>> wrote: >> >> Hi James, >> >> I like your patch. >> >> Do you know if other handlers are affected by a similar issue? In >> particular, the application:openFiles and application:printFiles >> also take references to NSObjects as their arguments. Could you >> please test if these handlers are affected or not? >> >> -- >> best regards, >> Anthony >> >> >> On 05/23/2013 10:56 PM, James Tomson wrote: >> >> Hi - this issue was originally discussed on the jdk7u-dev list here: >> >> http://mail.openjdk.java.net/__pipermail/jdk7u-dev/2013-May/__006446.html >> >> <http://mail.openjdk.java.net/pipermail/jdk7u-dev/2013-May/006446.html> >> >> Additionally a report should be available soon in the bug >> database as >> (JDK-8015302) >> http://bugs.sun.com/__bugdatabase/view_bug.do?bug___id=8015302 >> <http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8015302> >> >> To summarize, a bundled mac application which registers custom url >> schemes via the CFBundleURLSchemes entry in its Info.plist, and >> listens >> for uri events using >> com.apple.eawt.Application.__setOpenURIHandler, will >> not receive the URI used to launch the application. >> >> Once the application is running however, subsequent openURI >> events will >> be delivered without issue. The problem only manifests with the >> URI is >> used to launch the App initially. >> >> When the app is opened via URI, the following appears in the >> system log: >> >> ---------- >> JavaAppLauncher[74278]: -[NSAppleEventDescriptor >> paramDescriptorForKeyword:] called on invalid NSAppleEventDescriptor >> ---------- >> >> It appears that since the QueueingApplicationDelegate is only >> keeping >> references to those descriptor objects instead of making deep >> copies of >> them, >> the event descriptor for the initial URI that launches the app is >> invalidated by the time the app actually gets around to >> processing it. >> >> The following patch (same for both jdk8 and jdk7u sources) seems to >> resolve the issue: >> >> ---- >> diff --git >> a/src/macosx/native/sun/__osxapp/__QueuingApplicationDelegate.m >> b/src/macosx/native/sun/__osxapp/__QueuingApplicationDelegate.m >> --- a/src/macosx/native/sun/__osxapp/__QueuingApplicationDelegate.m >> +++ b/src/macosx/native/sun/__osxapp/__QueuingApplicationDelegate.m >> @@ -110,8 +110,14 @@ >> >> - (void)_handleOpenURLEvent:(__NSAppleEventDescriptor >> *)openURLEvent >> withReplyEvent:(__NSAppleEventDescriptor *)replyEvent >> { >> + // Make an explicit copy of the passed events as they may be >> invalidated by the time they're processed >> + NSAppleEventDescriptor *openURLEventCopy = [openURLEvent copy]; >> + NSAppleEventDescriptor *replyEventCopy = [replyEvent copy]; >> + >> [self.queue addObject:[^(){ >> - [self.realDelegate _handleOpenURLEvent:__openURLEvent >> withReplyEvent:replyEvent]; >> + [self.realDelegate _handleOpenURLEvent:__openURLEventCopy >> withReplyEvent:replyEventCopy]__; >> + [openURLEventCopy release]; >> + [replyEventCopy release]; >> } copy]]; >> } >> ----- >> >> Please let me know if there is additional information that I can >> provide >> - thanks! >> >> -James >> >>
