Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BatchFetchPolicy.clone() cause memory leak and ConcurrentModifcationException #1506

Closed
fataOT opened this issue May 4, 2022 · 1 comment · Fixed by #1707
Closed

BatchFetchPolicy.clone() cause memory leak and ConcurrentModifcationException #1506

fataOT opened this issue May 4, 2022 · 1 comment · Fixed by #1707

Comments

@fataOT
Copy link

fataOT commented May 4, 2022

The latest EL update to 2.7.10 introduced a memory leak within our application.
We think the root cause for this leads back to an old bug in BatchFetchPolicy.clone() (https://bugs.eclipse.org/bugs/show_bug.cgi?id=491349)

In 2.7.10 the initialization of the "dataResults" map has been shiftet to the constructor

    public BatchFetchPolicy(BatchFetchType type) {
        this.type = type;
        this.dataResults = new HashMap<Object, List<AbstractRecord>>();
        this.dataResults.put(this, new ArrayList<AbstractRecord>());
    }

So every created BatchFetchPolicy will have a dataResults-Map with the BatchFetchPolicy as an entry.

Now let's look at the clone() method

    public BatchFetchPolicy clone() {
        BatchFetchPolicy clone = null;
        try {
            clone = (BatchFetchPolicy)super.clone();
        } catch (CloneNotSupportedException error) {
            throw new InternalError(error.getMessage());
        }
        if (clone.dataResults != null) {
            clone.dataResults.put(clone, clone.dataResults.get(this));
        }
        return clone;
    }

The clone()-method will create a BatchFetchPolicy which will use the same map reference as the original BatchFetchPolicy.
The result of one clone() call is, that the dataResults map will have 2 entries, then 3, 4, ....
As a result of this, the garbage collection is unable to remove those objects.

The ConcurrentModifcationException from the referenced bug has the same origin.
The attributeExpressions list ist not a deep copy so all cloned BatchFetchPolicies will use the same list which can cause the ConcurrentModifcationException in case of heavy usage of QueryHints.BATCH within different threads/EntityManagers

Open questions:

  • What is the benefit for using the clone() method at all? Which data is necessary to remember?
  • Is there any reason to not deep copy the data?
  • Why is the dataResults map always initialized but only used for BatchFetchType.IN (we do not use BatchFetchType.IN at all, so our workaround for the memory leak is to only initialize the dataResults for BatchFetchType.IN)
@fataOT
Copy link
Author

fataOT commented Jul 13, 2022

The memory leak has already been fixed by #1245

rfelcman added a commit to rfelcman/eclipselink that referenced this issue Sep 19, 2022
Fixes eclipse-ee4j#1506 . This change will fix the ConcurrentModificationException of the attributeExpression list, when running the same (batch)query with different entity managers.

Signed-off-by: Radek Felcman <radek.felcman@oracle.com>
rfelcman added a commit to rfelcman/eclipselink that referenced this issue Sep 19, 2022
Fixes eclipse-ee4j#1506 . This change will fix the ConcurrentModificationException of the attributeExpression list, when running the same (batch)query with different entity managers.

Signed-off-by: Radek Felcman <radek.felcman@oracle.com>
lukasj pushed a commit that referenced this issue Oct 4, 2022
Fixes #1506 . This change will fix the ConcurrentModificationException of the attributeExpression list, when running the same (batch)query with different entity managers.

Signed-off-by: Radek Felcman <radek.felcman@oracle.com>
rfelcman added a commit that referenced this issue Jan 9, 2023
…om 2.7 (#1708)

Fixes #1506 . This change will fix the ConcurrentModificationException of the attributeExpression list, when running the same (batch)query with different entity managers.

Signed-off-by: Radek Felcman <radek.felcman@oracle.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant