RFR: 6543126: Level.known can leak memory

[Daniel Fuchs]

Hi Peter,

Rereading the comment and looking at the code I see
that the comment is actually right. There are two methods
with similar names:

Level.getLocalizedName() which can be overridden by subclasses
and Level.getLocalizedLevelName() which cannot.

By default Level.getLocalizedName() calls Level.getLocalizedLevelName(),
and  findByLocalizedLevelName() calls Level.getLocalizedLevelName() too,
not the overridable Level.getLocalizedName().

best regards,

-- daniel

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?
>
> Regards, Peter