RFR: 6543126: Level.known can leak memory
Daniel Fuchs
daniel.fuchs at oracle.com
Tue Aug 16 13:16:54 UTC 2016
On 16/08/16 13:59, Peter Levart wrote:
> Hi Daniel,
>
> Another place that is not clear to me is the following (in old code):
>
> 585 // Returns a KnownLevel with the given localized name matching
> 586 // by calling the Level.getLocalizedLevelName() method (i.e. found
> 587 // from the resourceBundle associated with the Level object).
> 588 // This method does not call Level.getLocalizedName() that may
> 589 // be overridden in a subclass implementation
> 590 static synchronized KnownLevel findByLocalizedLevelName(String name) {
> 591 for (List<KnownLevel> levels : nameToLevels.values()) {
> 592 for (KnownLevel l : levels) {
> 593 String lname = l.levelObject.getLocalizedLevelName();
> 594 if (name.equals(lname)) {
> 595 return l;
> 596 }
> 597 }
> 598 }
> 599 return null;
> 600 }
>
> In spite of claiming that "this method doesn't call
> Level.getLocalizedName() that may be overridden in subclass", it does
> just that. it calls the getLocalizedLevelName on the
> KnownLevel.levelObject, which is a user-constructed object. You changed
> the code into:
>
> 627 static synchronized Optional<Level>
> findByLocalizedLevelName(String name,
> 628 Function<KnownLevel, Stream<Level>> selector) {
> 629 purge();
> 630 return nameToLevels.values()
> 631 .stream()
> 632 .flatMap(List::stream)
> 633 .flatMap(selector)
> 634 .filter(lo ->
> name.equals(lo.getLocalizedLevelName()))
> 635 .findFirst();
> 636 }
>
> Which calls getLocalizedName() on the Level object selected by the given
> selector. In line 385 you pass in the selector to select the
> mirroredLevel, but in line 487, you pass in the selector to select the
> referent.
>
> So which one is the right one? If you want to obey the comment it should
> always be the mirroredLevel which is checked against localized name, right?
Good point - I suspect the comment is out of date.
Maybe Mandy will remember. I believe
Level.parse is supposed to call levelObject.getLocalizedName()
and Level.find is supposed to work with the referent.
Let me check.
best regards,
-- daniel
More information about the core-libs-dev
mailing list