Try #2, after running it through the the Xcode leak analyzer and unit testing (FYI keep your persistent domains lower case).
WebRev: http://cr.openjdk.java.net/~shemnon/RT-37788/webrev.01/rt.patch JIRA: https://javafx-jira.kenai.com/browse/RT-37788 And let’s move further discussion to the JIRA issue. On Jul 2, 2014, at 10:22 PM, Petr Pchelko <petr.pche...@oracle.com> wrote: > Hello, Danno. > > I’ve noticed that you are leaking userDefaults and expandedOptions objects. > Adding autorelease to them would save you some memory. > > With best regards. Petr. > >> On Jul 3, 2014, at 1:43 AM, Danno Ferrin <danno.fer...@oracle.com> wrote: >> >> Yes, this has to be fixed in native code. 8u40 it is then. >> >> I can make it cause a crash, but that starts with shooting myself in the >> foot, and not much can be done to fix it (by passing in bogus VM arguments). >> >> >> On Jul 2, 2014, at 3:40 PM, Stephen F Northover >> <steve.x.northo...@oracle.com> wrote: >> >>> Personally, I wouldn't change any native code at this point unless it was >>> fixing a crash. The review is for 8u40, correct? >>> >>> Steve >>> >>> On 2014-07-02, 5:38 PM, Chris Bensen wrote: >>>> I’m not sure about for 8u20. Seems fairly straight forward, and your Obj-C >>>> seems as good as any Obj-C. My only complaint at the moment is the >>>> following: >>>> 358 if ([pathParts count] > 2) { >>>> 359 // for 3 or more steps, the domain is first.second.third and >>>> the keys are "/first/second/third/", "fourth/", "fifth/"... etc >>>> 360 persistentDomain = [NSString stringWithFormat: @"%@.%@.%@", >>>> [pathParts objectAtIndex: 0], >>>> 361 [pathParts objectAtIndex: 1], [pathParts >>>> objectAtIndex: 2]]; >>>> 362 >>>> 363 [dictPath replaceObjectAtIndex: 0 withObject: [NSString >>>> stringWithFormat:@"/%@/%@/%@", [pathParts objectAtIndex: 0], >>>> 364 [pathParts >>>> objectAtIndex: 1], [pathParts objectAtIndex: 2]]]; >>>> 365 [dictPath removeObjectAtIndex: 2]; >>>> 366 [dictPath removeObjectAtIndex: 1]; >>>> 367 } else { >>>> 368 // for 1 or two steps, the domain is first.second.third and >>>> the keys are "/", "first/", "second/" >>>> 369 persistentDomain = @DEFAULT_JAVA_PREFS_DOMAIN; >>>> 370 [dictPath insertObject: @"" atIndex:0]; >>>> 371 } >>>> >>>> what if [pathParts count] is 0? I’d probably do a switch: >>>> >>>> switch ([pathParts count]) { >>>> case 0: >>>> //error >>>> return/break; >>>> case 1: >>>> case 2: >>>> 368 // for 1 or two steps, the domain is first.second.third and >>>> the keys are "/", "first/", "second/" >>>> 369 persistentDomain = @DEFAULT_JAVA_PREFS_DOMAIN; >>>> 370 [dictPath insertObject: @"" atIndex:0]; >>>> default: >>>> 359 // for 3 or more steps, the domain is first.second.third and >>>> the keys are "/first/second/third/", "fourth/", "fifth/"... etc >>>> 360 persistentDomain = [NSString stringWithFormat: @"%@.%@.%@", >>>> [pathParts objectAtIndex: 0], >>>> 361 [pathParts objectAtIndex: 1], [pathParts >>>> objectAtIndex: 2]]; >>>> 362 >>>> 363 [dictPath replaceObjectAtIndex: 0 withObject: [NSString >>>> stringWithFormat:@"/%@/%@/%@", [pathParts objectAtIndex: 0], >>>> 364 [pathParts >>>> objectAtIndex: 1], [pathParts objectAtIndex: 2]]]; >>>> 365 [dictPath removeObjectAtIndex: 2]; >>>> 366 [dictPath removeObjectAtIndex: 1]; >>>> >>>> } >>>> >>>> >>>> Make sense? Clear as mud? >>>> >>>> Chris >>>> >>>> >>>> On Jul 2, 2014, at 2:15 PM, Danno Ferrin <danno.fer...@oracle.com> wrote: >>>> >>>>> Chris, Kevin, Steve, >>>>> >>>>> Please review this fix for RT-37788. Since I am not an objective C any >>>>> comments are welcome. Also, please consider if this is too much for an >>>>> 8u20 fix (the diff is against the current 8u40 codebase). >>>>> >>>>> Webrev: http://cr.openjdk.java.net/~shemnon/RT-37788/webrev.00/ >>>>> JIRA: https://javafx-jira.kenai.com/browse/RT-37788 >>>>> >>>>> —Danno >
