Attachment #271946 for bug #522635

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

View | Details | Raw Unified | Return to bug 522635
Collapse All | Expand All




/*******************************************************************************
 * Copyright (c) 2017 Oracle and/or its affiliates. All rights reserved.
 * This program and the accompanying materials are made available under the
 * terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0
 * which accompanies this distribution.
 * The Eclipse Public License is available at http://www.eclipse.org/legal/epl-v10.html
 * and the Eclipse Distribution License is available at
 * http://www.eclipse.org/org/documents/edl-v10.php.
 *
 * Contributors:
 *     12/14/2017-3.0 Tomas Kraus
 *       - 522635: ConcurrentModificationException when triggering lazy load from conforming query
 ******************************************************************************/
package org.eclipse.persistence.testing.tests.identitymaps;

import java.util.Vector;

import org.eclipse.persistence.expressions.ExpressionBuilder;
import org.eclipse.persistence.queries.InMemoryQueryIndirectionPolicy;
import org.eclipse.persistence.queries.ReadAllQuery;
import org.eclipse.persistence.sessions.UnitOfWork;
import org.eclipse.persistence.testing.framework.TestCase;
import org.eclipse.persistence.testing.models.employee.domain.Employee;

/**
 * Bug# 522635 - Verify that {@code ConcurrentModificationException} is not thrown from {@code IdentityMapManager}
 * when {@code getAllFromIdentityMap} method is being called twice.
 */
public class GetAllFromIdentityMapTest extends TestCase {

    /** Current transaction. */
    private UnitOfWork uow = null;

    /** Query instance. */
    private ReadAllQuery query;

    /**
     * Create an instance of {@link ReadAllQuery} and initialize it.
     *
     * @param c entity class
     * @return new instance of initialized {@link ReadAllQuery}
     */
    private static ReadAllQuery newReadAllQuery(Class<?> c) {
        final ReadAllQuery query = new ReadAllQuery(c);
        query.conformResultsInUnitOfWork();
        query.setInMemoryQueryIndirectionPolicy(new InMemoryQueryIndirectionPolicy(InMemoryQueryIndirectionPolicy.SHOULD_TRIGGER_INDIRECTION));
        return query;
    }

    /**
     * Test setup.
     * Open transaction, initialize query and initialize cache with 1st query execution.
     */
    public void setup() {
        getSession().getIdentityMapAccessor().initializeAllIdentityMaps();
        uow = getSession().acquireUnitOfWork();
        query = newReadAllQuery(Employee.class);
        ExpressionBuilder emp = new ExpressionBuilder();
        query.setSelectionCriteria(emp.get("manager").get("firstName").equal("Bob"));
        uow.executeQuery(query);
    }

    /**
     * Test cleanup.
     * Release transaction and reset cache.
     */
    public void reset() {
        uow.release();
        uow = null;
        getSession().getIdentityMapAccessor().initializeAllIdentityMaps();
    }

    /**
     * Test case.
     * Verify that {@code ConcurrentModificationException} is not thrown from {@code IdentityMapManager}
     * during 2nd query execution when objects are already in the cache.
     */
    public void test() throws Throwable {
        Vector<Employee> employees = (Vector<Employee>)uow.executeQuery(query);
        assertFalse(employees.isEmpty());
        for (Employee employee : employees) {
            assertEquals("Bob", employee.getManager().getFirstName());
        }
    }

}

Lines 1-5 Link Here

(-)a/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/tests/identitymaps/IdentityMapTestSuite.java (-1 / +3 lines)
1	/*******************************************************************************	1	/*******************************************************************************
2	* Copyright (c) 1998, 2015 Oracle and/or its affiliates. All rights reserved.	2	* Copyright (c) 1998, 2017 Oracle and/or its affiliates. All rights reserved.
3	* This program and the accompanying materials are made available under the	3	* This program and the accompanying materials are made available under the
4	* terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0	4	* terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0
5	* which accompanies this distribution.	5	* which accompanies this distribution.
Lines 66-71 public void addTests() { Link Here
66	// Bug 5840635	66	// Bug 5840635
67	addTest(new CleanupCacheKeyCorrectnessTest());	67	addTest(new CleanupCacheKeyCorrectnessTest());
68	addTest(new TriggerValueHoldersSelfReferencingOneToOneTest());	68	addTest(new TriggerValueHoldersSelfReferencingOneToOneTest());
		69	// Bug 522635
		70	addTest(new GetAllFromIdentityMapTest());
69	}	71	}
70		72
71	private TestSuite getCacheIdentityMapSuite() {	73	private TestSuite getCacheIdentityMapSuite() {




/*******************************************************************************
 * Copyright (c) 2010, 2017 Oracle and/or its affiliates. All rights reserved.
 * This program and the accompanying materials are made available under the
 * terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0
 * which accompanies this distribution.

 * http://www.eclipse.org/org/documents/edl-v10.php.
 *
 * Contributors:
 *     08/17/2010-2.2 Tom Ware
 *       - 321041 ConcurrentModificationException on getFromIdentityMap() fix
 ******************************************************************************/
package org.eclipse.persistence.testing.tests.identitymaps;

import java.util.ConcurrentModificationException;

import org.eclipse.persistence.expressions.Expression;
import org.eclipse.persistence.expressions.ExpressionBuilder;
import org.eclipse.persistence.queries.ReadObjectQuery;
import org.eclipse.persistence.sessions.UnitOfWork;
import org.eclipse.persistence.testing.framework.TestCase;
import org.eclipse.persistence.testing.framework.TestErrorException;
import org.eclipse.persistence.testing.models.employee.domain.Employee;

/**
 * Bug# 321041 - EclipseLink throws ConcurrentModificationException when triggering lazy load from conforming query
 * @author tware
 *
 */
public class TriggerValueHoldersSelfReferencingOneToOneTest extends TestCase {

    /** Current transaction. */
    protected UnitOfWork uow = null;
    protected ConcurrentModificationException exception = null;

    /**
     * Test setup.
     * Open transaction, initialize query and initialize cache with 1st query executions.
     */
    public void setup() {
        getSession().getIdentityMapAccessor().initializeAllIdentityMaps();
        uow = getSession().acquireUnitOfWork();
        // Preload the UnitOfWork identity map with 2 Employees which have different managers

        // preload the UnitOfWork identity map with 2 Employees which have different managers
        ExpressionBuilder emp = new ExpressionBuilder();
        Expression queryExp = emp.get("firstName").equal("Charles").and(emp.get("lastName").equal("Chanley"));
        uow.readObject(Employee.class, queryExp);

        emp = new ExpressionBuilder();
        queryExp = emp.get("firstName").equal("Marcus").and(emp.get("lastName").equal("Saunders"));
        uow.readObject(Employee.class, queryExp);
    }

    /**
     * Test cleanup.
     * Release transaction and reset cache.
     */
    public void reset() {
        uow.release();
        uow = null;
        getSession().getIdentityMapAccessor().initializeAllIdentityMaps();
    }

    /**
     * Test case.
     * Verify that {@code ConcurrentModificationException} is not thrown from {@code IdentityMapManager}
     * during 2nd queries execution when objects are already in the cache.
     */
    public void test() {
        // We query for both Employees here because it is impossible to tell which order
        // keys will be returned from the identity map in
        // This bug only occurs when the first key returned is non-conforming and

        ReadObjectQuery query = new ReadObjectQuery(Employee.class, queryExp);
        query.conformResultsInUnitOfWork();
        query.getInMemoryQueryIndirectionPolicy().triggerIndirection();
        Employee bob = (Employee)uow.executeQuery(query);
        assertNotNull(bob);
        assertEquals("Bob", bob.getManager().getFirstName());
        emp = new ExpressionBuilder();
        queryExp = emp.get("manager").get("firstName").equal("John");
        query = new ReadObjectQuery(Employee.class, queryExp);
        query.conformResultsInUnitOfWork();
        query.getInMemoryQueryIndirectionPolicy().triggerIndirection();
        Employee john = (Employee)uow.executeQuery(query);
        assertNotNull(john);
        assertEquals("John", john.getManager().getFirstName());
    public void verify(){
        if (exception != null){
            throw new TestErrorException("A ConcurrentModificationException was thrown when using a Self Referencing Relationship and " +
                     "a TriggerIndirection query policy.");
        }
    }

    public void reset(){
        uow = null;
        getSession().getIdentityMapAccessor().initializeAllIdentityMaps();
    }

}




/*******************************************************************************
 * Copyright (c) 1998, 2017 Oracle and/or its affiliates. All rights reserved.
 * This program and the accompanying materials are made available under the
 * terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0
 * which accompanies this distribution.
 * The Eclipse Public License is available at http://www.eclipse.org/legal/epl-v10.html
 * and the Eclipse Distribution License is available at
 * http://www.eclipse.org/org/documents/edl-v10.php.
 *
 * Contributors:
 *     Oracle - initial API and implementation from Oracle TopLink
 *     12/14/2017-3.0 Tomas Kraus
 *       - 522635: ConcurrentModificationException when triggering lazy load from conforming query
 ******************************************************************************/
package org.eclipse.persistence.internal.identitymaps;

import java.util.Collection;
import java.util.Enumeration;
import java.util.Iterator;
import java.util.NoSuchElementException;

/**
 * Abstract {@link Enumeration} interface implementation for {@link IdentityMap}
 * interface. Allows to iterate over {@link CacheKey} instances stored in the map.
 *
 * @param <T> type of iterated {@link CacheKey} content
 */
public abstract class AbstractIdentityMapEnumeration<T> implements Enumeration<T> {

    /** {@link CacheKey} instances iterator. */
    protected final Iterator<CacheKey> cacheKeysIterator;

    /** Next key to be returned. */
    protected CacheKey nextKey;

    /** Value of {@code true} if readLocks should be checked or false otherwise. */
    protected boolean shouldCheckReadLocks;

    /**
     * Creates an instance of {@link CacheKey} content enumeration.
     *
     * @param keys {@link Collection} of {@link CacheKey} instances to be iterated
     * @param shouldCheckReadLocks value of {@code true} if read lock on the {@link CacheKey}
     *        instances should be checked or {@code false} otherwise
     */
    public AbstractIdentityMapEnumeration(Collection<CacheKey> keys, boolean shouldCheckReadLocks) {
        this.shouldCheckReadLocks = shouldCheckReadLocks;
        this.cacheKeysIterator = keys.iterator();
    }

    /**
     * Check whether this enumeration contains more elements.
     *
     * @return value of {@code true} if this enumeration object contains at least
     *         one more element to provide or {@code false} otherwise
     */
    @Override
    public boolean hasMoreElements() {
        this.nextKey = getNextCacheKey();
        return this.nextKey != null;
    }

    /**
     * Get next element of {@link CacheKey} content enumeration if this enumeration
     * object has at least one more element to provide.
     * It it expected that this method will be implemented using {@link #getNextElement()}
     * in child classes.
     *
     * @return the next element of this enumeration
     * @exception NoSuchElementException if no more elements exist
     */
    @Override
    public abstract T nextElement();

    /**
     * Get next {@link CacheKey} instance from iterator.
     *
     * @return next {@link CacheKey} instance or {@code null} if there is no more
     *         instance to provide
     */
    private CacheKey getNextCacheKey() {
        CacheKey key = null;
        while (cacheKeysIterator.hasNext() && (key == null)) {
            key = cacheKeysIterator.next();
        }
        return key;
    }

    /**
     * Get next element of {@link CacheKey} instances enumeration if this enumeration
     * object has at least one more element to provide.
     *
     * @return the next element of this enumeration
     * @exception NoSuchElementException  if no more elements exist
     */
    protected CacheKey getNextElement() {
        if (this.nextKey == null) {
            throw new NoSuchElementException("AbstractIdentityMapEnumeration nextElement");
        }
        // The read lock check is for avoidance of half built objects being returned.
        // bug 275724: Added shouldCheckReadLocks to avoid the read lock check when invalidating.
        if (shouldCheckReadLocks) {
            this.nextKey.checkReadLock();
        }
        return this.nextKey;
    }

}

Lines 1-5 Link Here

(-)a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/identitymaps/FullIdentityMap.java (-9 / +29 lines)
1	/*******************************************************************************	1	/*******************************************************************************
2	* Copyright (c) 1998, 2015 Oracle and/or its affiliates. All rights reserved.	2	* Copyright (c) 1998, 2017 Oracle and/or its affiliates. All rights reserved.
3	* This program and the accompanying materials are made available under the	3	* This program and the accompanying materials are made available under the
4	* terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0	4	* terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0
5	* which accompanies this distribution.	5	* which accompanies this distribution.
Lines 9-14 Link Here
9	*	9	*
10	* Contributors:	10	* Contributors:
11	* Oracle - initial API and implementation from Oracle TopLink	11	* Oracle - initial API and implementation from Oracle TopLink
		12	* 12/14/2017-3.0 Tomas Kraus
		13	* - 522635: ConcurrentModificationException when triggering lazy load from conforming query
12	******************************************************************************/	14	******************************************************************************/
13	package org.eclipse.persistence.internal.identitymaps;	15	package org.eclipse.persistence.internal.identitymaps;
14		16
Lines 84-94 public void collectLocks(HashMap threadList) { Link Here
84	}	86	}
85		87
86	/**	88	/**
87	* Allow for the cache to be iterated on.	89	* Allow for the cache {@link CacheKey#getObject()} elements to be iterated.
		90	*
		91	* @return {@link Enumeration} of {@link CacheKey#getObject()} instances.
88	*/	92	*/
89	@Override	93	@Override
90	public Enumeration elements() {	94	public Enumeration<Object> elements() {
91	return new IdentityMapEnumeration(this);	95	return new IdentityMapEnumeration(this.getCacheKeys().values());
92	}	96	}
93		97
94	/**	98	/**
Lines 157-172 public int getSize(Class myClass, boolean recurse) { Link Here
157	* Read locks will be checked.	161	* Read locks will be checked.
158	*/	162	*/
159	@Override	163	@Override
160	public Enumeration keys() {	164	public Enumeration<CacheKey> keys() {
161	return keys(true);	165	return keys(true);
162	}	166	}
163		167
164	/**	168	/**
165	* Allow for the cache keys to be iterated on.	169	* Allow for the {@link CacheKey} elements to be iterated.
166	* @param checkReadLocks - true if readLocks should be checked, false otherwise.	170	* {@link CacheKey} {@link Collection} is reused so this iteration may not be thread safe.
		171	*
		172	* @param checkReadLocks value of {@code true} if read lock on the {@link CacheKey}
		173	* instances should be checked or {@code false} otherwise
		174	* @return {@link Enumeration} of {@link CacheKey} instances.
		175	*/
		176	public Enumeration<CacheKey> keys(boolean checkReadLocks) {
		177	return new IdentityMapKeyEnumeration(this.getCacheKeys().values(), checkReadLocks);
		178	}
		179
		180	/**
		181	* Allow for the {@link CacheKey} elements to be iterated.
		182	* Clone of {@link CacheKey} {@link Collection} is returned so this iteration should
		183	* be thread safe.
		184	*
		185	* @return {@link Enumeration} with clone of the {@link CacheKey} {@link Collection}
167	*/	186	*/
168	public Enumeration keys(boolean checkReadLocks) {	187	@Override
169	return new IdentityMapKeyEnumeration(this, checkReadLocks);	188	public Enumeration<CacheKey> cloneKeys() {
		189	return new IdentityMapKeyEnumeration(new ArrayList(this.getCacheKeys().values()), true);
170	}	190	}
171		191
172	/**	192	/**

Lines 1-5 Link Here

(-)a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/identitymaps/IdentityMap.java (-4 / +14 lines)
1	/*******************************************************************************	1	/*******************************************************************************
2	* Copyright (c) 1998, 2015 Oracle and/or its affiliates. All rights reserved.	2	* Copyright (c) 1998, 2017 Oracle and/or its affiliates. All rights reserved.
3	* This program and the accompanying materials are made available under the	3	* This program and the accompanying materials are made available under the
4	* terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0	4	* terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0
5	* which accompanies this distribution.	5	* which accompanies this distribution.
Lines 9-14 Link Here
9	*	9	*
10	* Contributors:	10	* Contributors:
11	* Gordon Yorke - Part of the Cache Interceptor feature. (ER 219683)	11	* Gordon Yorke - Part of the Cache Interceptor feature. (ER 219683)
		12	* 12/14/2017-3.0 Tomas Kraus
		13	* - 522635: ConcurrentModificationException when triggering lazy load from conforming query
12	******************************************************************************/	14	******************************************************************************/
13	package org.eclipse.persistence.internal.identitymaps;	15	package org.eclipse.persistence.internal.identitymaps;
14		16
Lines 98-104 Link Here
98	/**	100	/**
99	* Allow for the cache to be iterated on.	101	* Allow for the cache to be iterated on.
100	*/	102	*/
101	public Enumeration elements();	103	public Enumeration<Object> elements();
102		104
103	/**	105	/**
104	* Return the object cached in the identity map or null if it could not be found.	106	* Return the object cached in the identity map or null if it could not be found.
Lines 181-193 Link Here
181	* Allow for the CacheKeys to be iterated on.	183	* Allow for the CacheKeys to be iterated on.
182	* Read locks should be checked	184	* Read locks should be checked
183	*/	185	*/
184	public Enumeration keys();	186	public Enumeration<CacheKey> keys();
		187
		188	/**
		189	* Allow for the CacheKeys to be iterated on using copy of keys enumeration.
		190	* This is thread safe access to keys.
		191	*
		192	* @return clone of the CacheKeys enumeration
		193	*/
		194	public Enumeration<CacheKey> cloneKeys();
185		195
186	/**	196	/**
187	* Allow for the CacheKeys to be iterated on.	197	* Allow for the CacheKeys to be iterated on.
188	* @param checkReadLocks - true if readLocks should be checked, false otherwise.	198	* @param checkReadLocks - true if readLocks should be checked, false otherwise.
189	*/	199	*/
190	public Enumeration keys(boolean checkReadLocks);	200	public Enumeration<CacheKey> keys(boolean checkReadLocks);
191		201
192	/**	202	/**
193	* Notify the cache that a lazy relationship has been triggered in the object	203	* Notify the cache that a lazy relationship has been triggered in the object

Lines 1-5 Link Here

(-)a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/identitymaps/IdentityMapEnumeration.java (-8 / +24 lines)
1	/*******************************************************************************	1	/*******************************************************************************
2	* Copyright (c) 1998, 2015 Oracle and/or its affiliates. All rights reserved.	2	* Copyright (c) 1998, 2017 Oracle and/or its affiliates. All rights reserved.
3	* This program and the accompanying materials are made available under the	3	* This program and the accompanying materials are made available under the
4	* terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0	4	* terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0
5	* which accompanies this distribution.	5	* which accompanies this distribution.
Lines 9-30 Link Here
9	*	9	*
10	* Contributors:	10	* Contributors:
11	* Oracle - initial API and implementation from Oracle TopLink	11	* Oracle - initial API and implementation from Oracle TopLink
		12	* 12/14/2017-3.0 Tomas Kraus
		13	* - 522635: ConcurrentModificationException when triggering lazy load from conforming query
12	******************************************************************************/	14	******************************************************************************/
13	package org.eclipse.persistence.internal.identitymaps;	15	package org.eclipse.persistence.internal.identitymaps;
14		16
		17	import java.util.Collection;
		18	import java.util.NoSuchElementException;
		19
15	/**	20	/**
16	* Used to allow iterating over a map.	21	* Allows to iterate over {@code Object} instances stored in {@link CacheKey} instances
		22	* in the {@link IdentityMap}.
17	*/	23	*/
18	public class IdentityMapEnumeration extends IdentityMapKeyEnumeration {	24	public class IdentityMapEnumeration extends AbstractIdentityMapEnumeration<Object>{
19	public IdentityMapEnumeration(FullIdentityMap map) {	25
20	super(map);	26	/**
		27	* Creates an instance of {@link CacheKey} instances enumeration.
		28	* Checking of read lock on the {@link CacheKey} instances is turned on.
		29	*
		30	* @param keys {@link Collection} of {@link CacheKey} instances to be iterated
		31	*/
		32	public IdentityMapEnumeration(Collection<CacheKey> keys) {
		33	super(keys, true);
21	}	34	}
22		35
23	/**	36	/**
24	* Return the nextElement. It must be set to null so that subsequent calls will	37	* Get next {@link CacheKey#getObject()} element of {@link CacheKey} enumeration
25	* actual get subsequent objects through the getNextCachedObject() logic.	38	* if this enumeration object has at least one more element to provide.
		39	*
		40	* @return the next {@link CacheKey#getObject()} element of this enumeration
		41	* @exception NoSuchElementException if no more elements exist
26	*/	42	*/
27	public Object nextElement() {	43	public Object nextElement() {
28	return ((CacheKey)super.nextElement()).getObject();	44	return getNextElement().getObject();
29	}	45	}
30	}	46	}




/*******************************************************************************
 * Copyright (c) 1998, 2017 Oracle and/or its affiliates. All rights reserved.
 * This program and the accompanying materials are made available under the
 * terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0
 * which accompanies this distribution.

 *
 * Contributors:
 *     Oracle - initial API and implementation from Oracle TopLink
 *     12/14/2017-3.0 Tomas Kraus
 *       - 522635: ConcurrentModificationException when triggering lazy load from conforming query
 ******************************************************************************/
package org.eclipse.persistence.internal.identitymaps;

import java.util.*;

/**
 * Allows to iterate over {@link CacheKey} instances stored in the {@link IdentityMap}.
 */
public class IdentityMapKeyEnumeration extends AbstractIdentityMapEnumeration<CacheKey> {

    /**
     * Creates an instance of {@link CacheKey} instances enumeration.
     * Checking of read lock on the {@link CacheKey} instances is turned on.
     *
     * @param keys {@link Collection} of {@link CacheKey} instances to be iterated
     */
    public IdentityMapKeyEnumeration(Collection<CacheKey> keys) {
        super(keys, true);

    public IdentityMapKeyEnumeration(FullIdentityMap map, boolean shouldCheckReadLocks) {
        this.map = map;
        this.shouldCheckReadLocks = shouldCheckReadLocks;
        this.cacheKeysIterator = map.getCacheKeys().values().iterator();
    }

    public boolean hasMoreElements() {
        this.nextKey = getNextCacheKey();
        return this.nextKey != null;
    }

    public Object nextElement() {
        if (this.nextKey == null) {
            throw new NoSuchElementException("IdentityMapKeyEnumeration nextElement");
        }

        // The read lock check is for avoidance of half built objects being returned.
        // bug 275724: Added shouldCheckReadLocks to avoid the read lock check when invalidating.
        if (shouldCheckReadLocks) {
            this.nextKey.checkReadLock();
        }
        return this.nextKey;
    }

    protected CacheKey getNextCacheKey() {
        CacheKey key = null;
        while (this.cacheKeysIterator.hasNext() && (key == null)) {
            key = (CacheKey)this.cacheKeysIterator.next();
        }
        return key;
    }

    /**
     * Creates an instance of {@link CacheKey} instances enumeration.
     *
     * @param keys {@link Collection} of {@link CacheKey} instances to be iterated
     * @param shouldCheckReadLocks value of {@code true} if read lock on the {@link CacheKey}
     *        instances should be checked or {@code false} otherwise
     */
    public IdentityMapKeyEnumeration(Collection<CacheKey> keys, boolean shouldCheckReadLocks) {
        super(keys, shouldCheckReadLocks);
    }

    /**
     * Get next element of {@link CacheKey} enumeration if this enumeration
     * object has at least one more element to provide.
     *
     * @return the next element of this enumeration
     * @exception  NoSuchElementException  if no more elements exist
     */
    @Override
    public CacheKey nextElement() {
        return getNextElement();
    }

}

Lines 12-18 Link Here

(-)a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/identitymaps/IdentityMapManager.java (-20 / +12 lines)
12	* Oracle - initial API and implementation from Oracle TopLink	12	* Oracle - initial API and implementation from Oracle TopLink
13	* Mark Wolochuk - Bug 321041 ConcurrentModificationException on getFromIdentityMap() fix	13	* Mark Wolochuk - Bug 321041 ConcurrentModificationException on getFromIdentityMap() fix
14	* 11/07/2017 - Dalia Abo Sheasha	14	* 11/07/2017 - Dalia Abo Sheasha
15	* - 526957 : Split the logging and trace messages	15	* - 526957: Split the logging and trace messages
		16	* 12/14/2017-3.0 Tomas Kraus
		17	* - 522635: ConcurrentModificationException when triggering lazy load from conforming query
16	******************************************************************************/	18	******************************************************************************/
17	package org.eclipse.persistence.internal.identitymaps;	19	package org.eclipse.persistence.internal.identitymaps;
18		20
Lines 539-550 public Vector getAllFromIdentityMap(Expression selectionCriteria, Class theClass Link Here
539	}	541	}
540	objects = new Vector();	542	objects = new Vector();
541	IdentityMap map = getIdentityMap(descriptor, false);	543	IdentityMap map = getIdentityMap(descriptor, false);
		544
		545	// Bug #522635 - if policy is set to trigger indirection, then iterate over a copy of the cache keys collection
		546	// to avoid a ConcurrentModificationException
		547	final Enumeration cacheEnum = valueHolderPolicy == InMemoryQueryIndirectionPolicy.SHOULD_TRIGGER_INDIRECTION ? map.cloneKeys() : map.keys();
		548
542	// bug 327900 - If don't read subclasses is set on the descriptor heed it.	549	// bug 327900 - If don't read subclasses is set on the descriptor heed it.
543	boolean readSubclassesOrNoInheritance = (!descriptor.hasInheritance() \|\| descriptor.getInheritancePolicy().shouldReadSubclasses());	550	boolean readSubclassesOrNoInheritance = (!descriptor.hasInheritance() \|\| descriptor.getInheritancePolicy().shouldReadSubclasses());
544		551
545	// cache the current time to avoid calculating it every time through the loop	552	// cache the current time to avoid calculating it every time through the loop
546	long currentTimeInMillis = System.currentTimeMillis();	553	long currentTimeInMillis = System.currentTimeMillis();
547	for (Enumeration cacheEnum = map.keys(); cacheEnum.hasMoreElements();) {	554	while (cacheEnum.hasMoreElements()) {
548	CacheKey key = (CacheKey)cacheEnum.nextElement();	555	CacheKey key = (CacheKey)cacheEnum.nextElement();
549	if ((key.getObject() == null) \|\| (!shouldReturnInvalidatedObjects && descriptor.getCacheInvalidationPolicy().isInvalidated(key, currentTimeInMillis))) {	556	if ((key.getObject() == null) \|\| (!shouldReturnInvalidatedObjects && descriptor.getCacheInvalidationPolicy().isInvalidated(key, currentTimeInMillis))) {
550	continue;	557	continue;
Lines 852-875 public Object getFromIdentityMap(Expression selectionCriteria, Class theClass, R Link Here
852	}	859	}
853	IdentityMap map = getIdentityMap(descriptor, false);	860	IdentityMap map = getIdentityMap(descriptor, false);
854		861
855	// Bug #321041 - if policy is set to trigger indirection, then make a copy of the cache keys collection	862	// Bug #321041 - if policy is set to trigger indirection, then iterate over a copy of the cache keys collection
856	// and iterate over that to avoid a ConcurrentModificationException.	863	// to avoid a ConcurrentModificationException
857	// This happens when the indirect attribute is of the same type (or has same mapped superclass) as	864	Enumeration cacheEnum = valueHolderPolicy == InMemoryQueryIndirectionPolicy.SHOULD_TRIGGER_INDIRECTION ? map.cloneKeys() : map.keys();
858	// the parent object. EclipseLink inserts the object into the same collection it is iterating over,
859	// which results in a ConcurrentModificationException.
860	// There's a slight performance hit in copying the collection, but we are already taking a hit
861	// by triggering indirection in the first place.
862	boolean copyKeyCollection = valueHolderPolicy == InMemoryQueryIndirectionPolicy.SHOULD_TRIGGER_INDIRECTION;
863	Vector cacheKeys = null;
864	if (copyKeyCollection) {
865	cacheKeys = new Vector(map.getSize());
866	for (Enumeration cacheEnum = map.keys(); cacheEnum.hasMoreElements();) {
867	CacheKey key = (CacheKey)cacheEnum.nextElement();
868	cacheKeys.add(key);
869	}
870	}
871
872	Enumeration cacheEnum = copyKeyCollection ? cacheKeys.elements() : map.keys();
873		865
874	// cache the current time to avoid calculating it every time through the loop	866	// cache the current time to avoid calculating it every time through the loop
875	long currentTimeInMillis = System.currentTimeMillis();	867	long currentTimeInMillis = System.currentTimeMillis();

Lines 1-5 Link Here

(-)a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/identitymaps/NoIdentityMap.java (-7 / +17 lines)
1	/*******************************************************************************	1	/*******************************************************************************
2	* Copyright (c) 1998, 2015 Oracle and/or its affiliates. All rights reserved.	2	* Copyright (c) 1998, 2017 Oracle and/or its affiliates. All rights reserved.
3	* This program and the accompanying materials are made available under the	3	* This program and the accompanying materials are made available under the
4	* terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0	4	* terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0
5	* which accompanies this distribution.	5	* which accompanies this distribution.
Lines 9-14 Link Here
9	*	9	*
10	* Contributors:	10	* Contributors:
11	* Oracle - initial API and implementation from Oracle TopLink	11	* Oracle - initial API and implementation from Oracle TopLink
		12	* 12/14/2017-3.0 Tomas Kraus
		13	* - 522635: ConcurrentModificationException when triggering lazy load from conforming query
12	******************************************************************************/	14	******************************************************************************/
13	package org.eclipse.persistence.internal.identitymaps;	15	package org.eclipse.persistence.internal.identitymaps;
14		16
Lines 45-52 public void collectLocks(HashMap threadList) { Link Here
45	* Return an empty enumerator.	47	* Return an empty enumerator.
46	*/	48	*/
47	@Override	49	@Override
48	public Enumeration elements() {	50	public Enumeration<Object> elements() {
49	return new Vector(0).elements();	51	return Collections.<Object>emptyEnumeration();
50	}	52	}
51		53
52	/**	54	/**
Lines 101-115 public Object getWriteLockValue(Object primaryKey) { Link Here
101	* Return an empty enumerator.	103	* Return an empty enumerator.
102	*/	104	*/
103	@Override	105	@Override
104	public Enumeration keys() {	106	public Enumeration<CacheKey> keys() {
105	return new Vector(0).elements();	107	return Collections.<CacheKey>emptyEnumeration();
106	}	108	}
107		109
108	/**	110	/**
109	* Return an empty enumerator.	111	* Return an empty enumerator.
110	*/	112	*/
111	public Enumeration keys(boolean checkReadLocks) {	113	@Override
112	return keys();	114	public Enumeration<CacheKey> cloneKeys() {
		115	return Collections.<CacheKey>emptyEnumeration();
		116	}
		117
		118	/**
		119	* Return an empty enumerator.
		120	*/
		121	public Enumeration<CacheKey> keys(boolean checkReadLocks) {
		122	return Collections.<CacheKey>emptyEnumeration();
113	}	123	}
114		124
115	/**	125	/**

Lines 1-5 Link Here

(-)a/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/sessions/interceptors/CacheInterceptor.java (-8 / +28 lines)
1	/*******************************************************************************	1	/*******************************************************************************
2	* Copyright (c) 1998, 2015 Oracle and/or its affiliates. All rights reserved.	2	* Copyright (c) 1998, 2017 Oracle and/or its affiliates. All rights reserved.
3	* This program and the accompanying materials are made available under the	3	* This program and the accompanying materials are made available under the
4	* terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0	4	* terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0
5	* which accompanies this distribution.	5	* which accompanies this distribution.
Lines 9-17 Link Here
9	*	9	*
10	* Contributors:	10	* Contributors:
11	* Gordon Yorke - Initial Feature development	11	* Gordon Yorke - Initial Feature development
		12	* 12/14/2017-3.0 Tomas Kraus
		13	* - 522635: ConcurrentModificationException when triggering lazy load from conforming query
12	******************************************************************************/	14	******************************************************************************/
13	package org.eclipse.persistence.sessions.interceptors;	15	package org.eclipse.persistence.sessions.interceptors;
14		16
		17	import java.util.Collection;
15	import java.util.Enumeration;	18	import java.util.Enumeration;
16	import java.util.HashMap;	19	import java.util.HashMap;
17	import java.util.Map;	20	import java.util.Map;
Lines 152-158 public boolean containsKey(Object primaryKey) { Link Here
152	* Allow for the cache to be iterated on. This method is only used during debugging when	155	* Allow for the cache to be iterated on. This method is only used during debugging when
153	* validateCache() has been called to print out the contents of the cache.	156	* validateCache() has been called to print out the contents of the cache.
154	*/	157	*/
155	public Enumeration elements() {	158	public Enumeration<Object> elements() {
156	return this.targetIdentityMap.elements();	159	return this.targetIdentityMap.elements();
157	}	160	}
158		161
Lines 262-278 public Object getWriteLockValue(Object primaryKey) { Link Here
262	}	265	}
263		266
264	/**	267	/**
265	* Allow for the CacheKeys to be iterated on.	268	* Allow for the {@link CacheKey} elements to be iterated.
		269	* {@link CacheKey} {@link Collection} is reused so this iteration may not be thread safe.
		270	*
		271	* @return {@link Enumeration} of {@link CacheKey} instances.
266	*/	272	*/
267	public Enumeration keys() {	273	public Enumeration<CacheKey> keys() {
268	return this.targetIdentityMap.keys();	274	return this.targetIdentityMap.keys();
269	}	275	}
270		276
271	/**	277	/**
272	* Allow for the CacheKeys to be iterated on. - value should be true	278	* Allow for the {@link CacheKey} elements to be iterated.
273	* if readloacks are to be used, false otherwise.	279	* Clone of {@link CacheKey} {@link Collection} is returned so this iteration should
		280	* be thread safe.
		281	*
		282	* @return {@link Enumeration} with clone of the {@link CacheKey} {@link Collection}
274	*/	283	*/
275	public Enumeration keys(boolean checkReadLocks) {	284	public Enumeration<CacheKey> cloneKeys() {
		285	return this.targetIdentityMap.cloneKeys();
		286	}
		287
		288	/**
		289	* Allow for the {@link CacheKey} elements to be iterated.
		290	* {@link CacheKey} {@link Collection} is reused so this iteration may not be thread safe.
		291	*
		292	* @param checkReadLocks value of {@code true} if read lock on the {@link CacheKey}
		293	* instances should be checked or {@code false} otherwise
		294	* @return {@link Enumeration} of {@link CacheKey} instances.
		295	*/
		296	public Enumeration<CacheKey> keys(boolean checkReadLocks) {
276	return this.targetIdentityMap.keys(checkReadLocks);	297	return this.targetIdentityMap.keys(checkReadLocks);
277	}	298	}
278		299
279	-

Return to bug 522635

Added Link Here

(-)a/foundation/eclipselink.core.test/src/org/eclipse/persistence/testing/tests/identitymaps/GetAllFromIdentityMapTest.java (+86 lines)
1	/*******************************************************************************
2	* Copyright (c) 2017 Oracle and/or its affiliates. All rights reserved.
3	* This program and the accompanying materials are made available under the
4	* terms of the Eclipse Public License v1.0 and Eclipse Distribution License v. 1.0
5	* which accompanies this distribution.
6	* The Eclipse Public License is available at http://www.eclipse.org/legal/epl-v10.html
7	* and the Eclipse Distribution License is available at
8	* http://www.eclipse.org/org/documents/edl-v10.php.
9	*
10	* Contributors:
11	* 12/14/2017-3.0 Tomas Kraus
12	* - 522635: ConcurrentModificationException when triggering lazy load from conforming query
13	******************************************************************************/
14	package org.eclipse.persistence.testing.tests.identitymaps;
15
16	import java.util.Vector;
17
18	import org.eclipse.persistence.expressions.ExpressionBuilder;
19	import org.eclipse.persistence.queries.InMemoryQueryIndirectionPolicy;
20	import org.eclipse.persistence.queries.ReadAllQuery;
21	import org.eclipse.persistence.sessions.UnitOfWork;
22	import org.eclipse.persistence.testing.framework.TestCase;
23	import org.eclipse.persistence.testing.models.employee.domain.Employee;
24
25	/**
26	* Bug# 522635 - Verify that {@code ConcurrentModificationException} is not thrown from {@code IdentityMapManager}
27	* when {@code getAllFromIdentityMap} method is being called twice.
28	*/
29	public class GetAllFromIdentityMapTest extends TestCase {
30
31	/** Current transaction. */
32	private UnitOfWork uow = null;
33
34	/** Query instance. */
35	private ReadAllQuery query;
36
37	/**
38	* Create an instance of {@link ReadAllQuery} and initialize it.
39	*
40	* @param c entity class
41	* @return new instance of initialized {@link ReadAllQuery}
42	*/
43	private static ReadAllQuery newReadAllQuery(Class<?> c) {
44	final ReadAllQuery query = new ReadAllQuery(c);
45	query.conformResultsInUnitOfWork();
46	query.setInMemoryQueryIndirectionPolicy(new InMemoryQueryIndirectionPolicy(InMemoryQueryIndirectionPolicy.SHOULD_TRIGGER_INDIRECTION));
47	return query;
48	}
49
50	/**
51	* Test setup.
52	* Open transaction, initialize query and initialize cache with 1st query execution.
53	*/
54	public void setup() {
55	getSession().getIdentityMapAccessor().initializeAllIdentityMaps();
56	uow = getSession().acquireUnitOfWork();
57	query = newReadAllQuery(Employee.class);
58	ExpressionBuilder emp = new ExpressionBuilder();
59	query.setSelectionCriteria(emp.get("manager").get("firstName").equal("Bob"));
60	uow.executeQuery(query);
61	}
62
63	/**
64	* Test cleanup.
65	* Release transaction and reset cache.
66	*/
67	public void reset() {
68	uow.release();
69	uow = null;
70	getSession().getIdentityMapAccessor().initializeAllIdentityMaps();
71	}
72
73	/**
74	* Test case.
75	* Verify that {@code ConcurrentModificationException} is not thrown from {@code IdentityMapManager}
76	* during 2nd query execution when objects are already in the cache.
77	*/
78	public void test() throws Throwable {
79	Vector<Employee> employees = (Vector<Employee>)uow.executeQuery(query);
80	assertFalse(employees.isEmpty());
81	for (Employee employee : employees) {
82	assertEquals("Bob", employee.getManager().getFirstName());
83	}
84	}
85
86	}