Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
[eclipselink-dev] Major issue with new object introduced by revision 2618

Major issue with new object introduced by revision 2618

 

In UnitOfWorkImpl.java, method -> registerNewObject(Object implementation, ClassDescriptor descriptor)

 

Old line: Object original = builder.buildNewInstance();

 

New line: Object original = null;

 

Then we have lot of code relying on having something else than null for original.

 

For a start we have couple of lines later in registerNewObject this call:

 

registerNewObjectClone(implementation, original, descriptor); //this method calls registerNewObjectInIdentityMap

 

A reviewer should have had some doubt on passing null on a method used to a real value. I think we should restrict revision to less unrelated code changes so it is more likely to be seriously reviewed.

 

You can read this in registerNewObjectClone:

protected void registerNewObjectClone(Object clone, Object original, ClassDescriptor descriptor) {

        // Check if the new objects should be cached.

        registerNewObjectInIdentityMap(clone, original, descriptor);

 

        getNewObjectsCloneToOriginal().put(clone, original);

        getNewObjectsOriginalToClone().put(original, clone);

 

getNewObjectsOriginalToClone return a IdentityHashMap. IdentityHashMap create a fake null key when null is passed in.

 

So you can already guess that each subsequent call to registerNewObjectClone will replace previous value because will have same fake null key. That already looks suspicious.

 

Then we have many clients of this incomplete Map, which is having a random unique entry:

 

The most impacted client I found so far is:

 

/**

     * INTERNAL:

     * Return any new objects matching the _expression_.

     * Used for in-memory querying.

     */

    public Vector getAllFromNewObjects(_expression_ selectionCriteria, Class theClass, AbstractRecord translationRow, int valueHolderPolicy) {

        //If new object are in the cache then they will have already been queried.

        //bug 4585129: in findAll() case, (i.e. no selection criteria), we do not do the optimization query.

        if (shouldNewObjectsBeCached() && (selectionCriteria != null)) {

            return new Vector(1);

        }

 

        // PERF: Avoid initialization of new objects if none.

        if (!hasNewObjects()) {

            return new Vector(1);

        }

 

        Vector objects = new Vector();

        for (Iterator newObjectsEnum = getNewObjectsOriginalToClone().values().iterator();

                 newObjectsEnum.hasNext();) {

            Object object = newObjectsEnum.next();

            if (theClass.isInstance(object)) {

                if (selectionCriteria == null) {

                    objects.addElement(object);

                } else if (selectionCriteria.doesConform(object, this, translationRow, valueHolderPolicy)) {

                    objects.addElement(object);

                }

            }

        }

        return objects;

    }

 

Please fix ASAP in 1.1.2 and build a milestone, we are stuck without this basic functionality: https://bugs.eclipse.org/bugs/show_bug.cgi?id=277205

 


Back to the top