You are viewing a plain text version of this content. The canonical link for it is here.
Posted to slide-dev@jakarta.apache.org by Martin Kalén <mk...@apache.org> on 2007/06/27 21:24:40 UTC

Problem with PropertyName equals() when using JNDIPrincipalStore

Greetings developers,
  after some hours of debugging a weird phenomena in my application I
have a question about String .intern() usage in the equals() method of
org.apache.slide.common.PropertyName.


Quick summary:

  * problem with retrieving properties from a NodeRevisionDescriptor,
    when updating org.apache.slide.common.PropertyName to trunk
    (SVN r429274)

  * problem goes away if reverting change to PropertyName#equals,
    changing line 179 from:

     return this.name == that.name && this.namespace == that.namespace;

    to:

     return this.name.equals(that.name) &&
            this.namespace.equals(that.namespace);

    (ie backing out r429274)


Background information:

  I am using org.apache.slide.store.txjndi.JNDIPrincipalStore for
retrieving user and group information from an LDAP server. When updating
Slide from SVN trunk my groups started loosing all it's members [1]
after some initial time - everything works fine at first
NodeRevisionDescriptor property access, but not on subsequent property
reads.

Metacode for my usecase (based on the repository block from Cocoon
v2.1.6):

[code]
  public static final PropertyName GROUP_MEMBER_SET =
         PropertyName.getPropertyName("group-member-set");

...

  SlideToken slideToken =
    new SlideTokenImpl(new CredentialsToken(caller));

  Structure structure = nat.getStructureHelper();
  Content content = nat.getContentHelper();
  ObjectNode object = structure.retrieve(slideToken,path);
  String uri = object.getUri();

  NodeRevisionDescriptors revisions =
   content.retrieve(slideToken, uri);

  NodeRevisionDescriptor revision =
   content.retrieve(slideToken, revisions);

  NodeProperty property =
   revision.getProperty(GROUP_MEMBER_SET);
[/code]

The last line tries to get the DAV:group-member-set property from the
NodeRevisionDescriptor. This works fine on first access, but after a
subsequent update of the revision descriptor due to JNDIPrincipalStore
executing it's background RefreshThread to refresh the descriptor from
LDAP, the PropertyName for DAV:group-member-set used for access is no
longer considered equal to the PropertyName for DAV:group-member-set
in "properties" hashtable of the NodeRevisionDescriptor (ie null is
returned for the "property" reference above, although memory
inspection shows that the NodeRevisionDescriptor indeed has the
DAV:group-member-set property in it's property table).

If i back out SVN commit r429274 [2] everything works fine. Reading up
a bit on String.intern() does not make me any wiser, since I think it
ought to work with "==" comparision in PropertyName, but I have a go
vs no-go reproducable system showing that "==" and String equals() does
not behave the same.

Anyone else that had similar problems? Thoughts about this? Would it be
OK to back out r429274?

The SVN comment states performance improvements as reason for changing
"String.equals()" to "==", but I suspect that in recent Java runtimes
this difference might be almost neglectable.

Best regards,
  Martin

[1] group->member mapping is implemented by <href/>-values of the
"DAV:group-member-set" property in JNDIPrincipalStore

[2] See 
http://mail-archives.apache.org/mod_mbox/jakarta-slide-dev/200608.mbox/%3c20060807083007.E0E291A981A@eris.apache.org%3e

---------------------------------------------------------------------
To unsubscribe, e-mail: slide-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: slide-dev-help@jakarta.apache.org


Re: Problem with PropertyName equals() when using JNDIPrincipalStore

Posted by Roland Weber <os...@dubioso.net>.
Martin Kalén wrote:
> in the Slide
> class PropertyName, all the String objects are interned in the
> constructor.

That sounds like a potential memory leak to me.

> The Java specification states that two interned strings a and b are
> guaranteed to give a==b true when a.equals(b) is true.

Then either the intern'd strings keep piling up in memory, or
at some point the JVM decides to throw some of them out and
the == begins to fail.

cheers,
  Roland


---------------------------------------------------------------------
To unsubscribe, e-mail: slide-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: slide-dev-help@jakarta.apache.org


RE: Problem with PropertyName equals() when using JNDIPrincipalStore

Posted by Miguel Figueiredo <mf...@maisis.pt>.
Hello Martin,

 I see your point, in fact I was not aware of the String#intern() behaviour.
Thank you for the links: learned something new, although somewhat unsafe to
use :)

Best regards,
Miguel Figueiredo

-----Original Message-----
From: Martin Kalén [mailto:mkalen@apache.org] 
Sent: quinta-feira, 28 de Junho de 2007 10:16
To: Slide Developers Mailing List
Subject: Re: Problem with PropertyName equals() when using
JNDIPrincipalStore


Thanks for reviewing Miguel, and yes - that was a small needle and a
big haystack. :)

Miguel Figueiredo wrote:
>  Since we are in Java, the default behaviour of the == operator is memory
> address comparison. Seems that it's possible to overload operators for any
> object, but in the String class the '==' is not overloaded.
>  It's strange how the committer let this go through, but he was right on
the
> performance statement: it's faster to compare 2 string addresses than the
> actual content of the strings : )
>  I would say it's safe to revert.

It's actually not as bad as missing an '==' overload - in the Slide
class PropertyName, all the String objects are interned in the
constructor.

The Java specification states that two interned strings a and b are
guaranteed to give a==b true when a.equals(b) is true.

So in theory it should work...
See also:
http://java.sun.com/j2se/1.5.0/docs/api/java/lang/String.html#intern()
and:
http://www.javaworld.com/javaworld/javaqa/2003-12/01-qa-1212-intern.html

My suspicion is that the background thread used for refreshing nodes in
JNDIPrincipalStore (JNDIPrincipalStore$RefreshThread) is the culprit,
and that the above statement for .intern() Strings is no longer true if
running under multithreaded conditions in the JRE?

It would be happy to get some clues as how to prove this, the only thing
I could do in the end was to see that two PropertyName objects with the
same namespace and name but different object references were not
considered equal with "==" comparision.

Anyhow, before someone can explain exactly why it happens I think that
reverting the checkin is OK as long as the measured performance
improvement was not a considerable amount. Also, I think that it is more
safe to first have correct behaviour under all JRE conditions and then
work on improving the PropertyName performance.

Regards,
  Martin


---------------------------------------------------------------------
To unsubscribe, e-mail: slide-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: slide-dev-help@jakarta.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: slide-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: slide-dev-help@jakarta.apache.org


Re: Problem with PropertyName equals() when using JNDIPrincipalStore

Posted by Martin Kalén <mk...@apache.org>.
Thanks for reviewing Miguel, and yes - that was a small needle and a
big haystack. :)

Miguel Figueiredo wrote:
>  Since we are in Java, the default behaviour of the == operator is memory
> address comparison. Seems that it's possible to overload operators for any
> object, but in the String class the '==' is not overloaded.
>  It's strange how the committer let this go through, but he was right on the
> performance statement: it's faster to compare 2 string addresses than the
> actual content of the strings : )
>  I would say it's safe to revert.

It's actually not as bad as missing an '==' overload - in the Slide
class PropertyName, all the String objects are interned in the
constructor.

The Java specification states that two interned strings a and b are
guaranteed to give a==b true when a.equals(b) is true.

So in theory it should work...
See also:
http://java.sun.com/j2se/1.5.0/docs/api/java/lang/String.html#intern()
and:
http://www.javaworld.com/javaworld/javaqa/2003-12/01-qa-1212-intern.html

My suspicion is that the background thread used for refreshing nodes in
JNDIPrincipalStore (JNDIPrincipalStore$RefreshThread) is the culprit,
and that the above statement for .intern() Strings is no longer true if
running under multithreaded conditions in the JRE?

It would be happy to get some clues as how to prove this, the only thing
I could do in the end was to see that two PropertyName objects with the
same namespace and name but different object references were not
considered equal with "==" comparision.

Anyhow, before someone can explain exactly why it happens I think that
reverting the checkin is OK as long as the measured performance
improvement was not a considerable amount. Also, I think that it is more
safe to first have correct behaviour under all JRE conditions and then
work on improving the PropertyName performance.

Regards,
  Martin


---------------------------------------------------------------------
To unsubscribe, e-mail: slide-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: slide-dev-help@jakarta.apache.org


RE: Problem with PropertyName equals() when using JNDIPrincipalStore

Posted by Miguel Figueiredo <mf...@maisis.pt>.
Hello Martin,

 Nice needle in the ashtray you found over there btw.
 Since we are in Java, the default behaviour of the == operator is memory
address comparison. Seems that it's possible to overload operators for any
object, but in the String class the '==' is not overloaded.
 It's strange how the committer let this go through, but he was right on the
performance statement: it's faster to compare 2 string addresses than the
actual content of the strings : )
 I would say it's safe to revert.

 Best Regards,
 Miguel Figueiredo


-----Original Message-----
From: Martin Kalén [mailto:mkalen@apache.org] 
Sent: quarta-feira, 27 de Junho de 2007 20:25
To: slide-dev@jakarta.apache.org
Subject: Problem with PropertyName equals() when using JNDIPrincipalStore

Greetings developers,
  after some hours of debugging a weird phenomena in my application I
have a question about String .intern() usage in the equals() method of
org.apache.slide.common.PropertyName.


Quick summary:

  * problem with retrieving properties from a NodeRevisionDescriptor,
    when updating org.apache.slide.common.PropertyName to trunk
    (SVN r429274)

  * problem goes away if reverting change to PropertyName#equals,
    changing line 179 from:

     return this.name == that.name && this.namespace == that.namespace;

    to:

     return this.name.equals(that.name) &&
            this.namespace.equals(that.namespace);

    (ie backing out r429274)


Background information:

  I am using org.apache.slide.store.txjndi.JNDIPrincipalStore for
retrieving user and group information from an LDAP server. When updating
Slide from SVN trunk my groups started loosing all it's members [1]
after some initial time - everything works fine at first
NodeRevisionDescriptor property access, but not on subsequent property
reads.

Metacode for my usecase (based on the repository block from Cocoon
v2.1.6):

[code]
  public static final PropertyName GROUP_MEMBER_SET =
         PropertyName.getPropertyName("group-member-set");

...

  SlideToken slideToken =
    new SlideTokenImpl(new CredentialsToken(caller));

  Structure structure = nat.getStructureHelper();
  Content content = nat.getContentHelper();
  ObjectNode object = structure.retrieve(slideToken,path);
  String uri = object.getUri();

  NodeRevisionDescriptors revisions =
   content.retrieve(slideToken, uri);

  NodeRevisionDescriptor revision =
   content.retrieve(slideToken, revisions);

  NodeProperty property =
   revision.getProperty(GROUP_MEMBER_SET);
[/code]

The last line tries to get the DAV:group-member-set property from the
NodeRevisionDescriptor. This works fine on first access, but after a
subsequent update of the revision descriptor due to JNDIPrincipalStore
executing it's background RefreshThread to refresh the descriptor from
LDAP, the PropertyName for DAV:group-member-set used for access is no
longer considered equal to the PropertyName for DAV:group-member-set
in "properties" hashtable of the NodeRevisionDescriptor (ie null is
returned for the "property" reference above, although memory
inspection shows that the NodeRevisionDescriptor indeed has the
DAV:group-member-set property in it's property table).

If i back out SVN commit r429274 [2] everything works fine. Reading up
a bit on String.intern() does not make me any wiser, since I think it
ought to work with "==" comparision in PropertyName, but I have a go
vs no-go reproducable system showing that "==" and String equals() does
not behave the same.

Anyone else that had similar problems? Thoughts about this? Would it be
OK to back out r429274?

The SVN comment states performance improvements as reason for changing
"String.equals()" to "==", but I suspect that in recent Java runtimes
this difference might be almost neglectable.

Best regards,
  Martin

[1] group->member mapping is implemented by <href/>-values of the
"DAV:group-member-set" property in JNDIPrincipalStore

[2] See 
http://mail-archives.apache.org/mod_mbox/jakarta-slide-dev/200608.mbox/%3c20
060807083007.E0E291A981A@eris.apache.org%3e

---------------------------------------------------------------------
To unsubscribe, e-mail: slide-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: slide-dev-help@jakarta.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: slide-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: slide-dev-help@jakarta.apache.org