[#CORE-3297] DatastoreIdentity : OID.hashCode() use of toString() can lead to incorrect identification of duplicate objects - DataNucleus JIRA

JPOX Core (ARCHIVED)

DatastoreIdentity : OID.hashCode() use of toString() can lead to incorrect identification of duplicate objects

Created: 12/Jun/07 06:14 PM Updated: 12/Dec/07 11:14 AM Resolved: 09/Oct/07 09:23 AM

Component/s:

None

Affects Version/s:

1.1.6, 1.1.7, 1.1.8, 1.2.0-beta-1, 1.2.0-beta-2, 1.2.0-beta-3

Fix Version/s:

1.1.9, 1.2.0-beta-4

Description

« Hide

I have two objects with the same hashcode:

"71787[OID]org.apache.maven.continuum.model.scm.ChangeFile"
"17260[OID]org.apache.maven.continuum.model.project.ProjectDependency"

These clash in the level 1 cache, resulting in an attempt to get the second giving back the first.

I believe the cache should use a combination of oid int & the class name, eg:
71787 * 39 + "org.apache.maven.continuum.model.scm.ChangeFile".hashCode()

It should also check that the object it gets back is the one expected, in AbstractPersistenceManager line 3906 from jpox 1.1.6, something like:

if ( !sm.myID.equals( id ) )
{
sm = null;
}

Sort Order:

[ Permalink | « Hide ]

Brett Porter added a comment - 12/Jun/07 07:17 PM

actually, the code supplied doesn't work, as the hashcode is used to test equality, which leads to them being "equal". So the hashcode function must be reviewed.

[ Permalink | « Hide ]

Brett Porter added a comment - 12/Jun/07 07:59 PM

I've also found this to effect DetachState

[ Permalink | « Hide ]

Brett Porter added a comment - 12/Jun/07 08:28 PM

ok, changing the assignment of hashcode to the method above in OID corrects the problem for me.

The way detachstate works may need to be reviewed since that map is vulnerable to any further collisions.

I haven't looked at 1.2.0-beta yet to see if these problems are irrelevant there.

[ Permalink | « Hide ]

Andy Jefferson added a comment - 13/Jun/07 12:09 AM

All comments should be around CVS HEAD. We just dont have time for 1.1 support. Thx

[ Permalink | « Hide ]

Brett Porter added a comment - 13/Jun/07 04:09 AM

a cursory glance at HEAD shows the same code in place, so I expect it to suffer the exact same problem.

[ Permalink | « Hide ]

Andy Jefferson added a comment - 13/Jun/07 07:16 AM

CVS HEAD now has a more conventional form of equals and hashCode. Since no testcase was provided you'll have to test if this works for you. If you come backe before 17/06 and confirm that this passes your tests then it could be backported onto 1.1 (the final planned 1.1 release is on 18/06)

[ Permalink | « Hide ]

Andy Jefferson added a comment - 13/Jun/07 04:17 PM

rolled back that change - JDO TCK fails.

[ Permalink | « Hide ]

Andy Jefferson added a comment - 27/Jul/07 11:33 AM

CVS HEAD now has contrib from Laurent Balthasar for equals() that should fix this. Consequently we assume it's fixed