DeadLock, DeadLock diagnostic and CacheKey NPE

This PR aggregates following PRs into single one: DeadLock diagnostic fix - Lock map snapshots eclipse-ee4j#2288 Cache DeadLock Detection test move from org.eclipse.persistence.jpa.jse.test into new org.eclipse.persistence.jpa.testapps.deadlock.diagnostic Maven module eclipse-ee4j#2304 New eclipselink dead lock scenario ... enhancement and unit test eclipse-ee4j#2350 CacheKey.equals() NPE bugfix plus unit test eclipse-ee4j#2364 Signed-off-by: Radek Felcman <[email protected]>
rfelcman · Feb 26, 2025 · 51c09f6 · 51c09f6
1 parent a8a439b
commit 51c09f6
Show file tree

Hide file tree

Showing 44 changed files with 2,123 additions and 339 deletions.
diff --git a/antbuild.properties b/antbuild.properties
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2008, 2024 Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2008, 2025 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 v. 2.0 which is available at
@@ -111,6 +111,7 @@ eclipselink.jpa.spring.test=${eclipselink.jpa.base}/eclipselink.jpa.spring.test
 eclipselink.jpa.wdf.test=${eclipselink.jpa.base}/eclipselink.jpa.wdf.test
 eclipselink.jpa.xsds=${eclipselink.jpa}/resource/org/eclipse/persistence/jpa
 eclipselink.jpa.plugins=${eclipselink.jpa.base}/${plugins.dir}
+eclipselink.deadlock.test=${eclipselink.jpa.base}/eclipselink.jpa.test.deadlock
 eclipselink.modelgen=${eclipselink.jpa.base}/org.eclipse.persistence.jpa.modelgen
 eclipselink.jpars=${eclipselink.jpa.base}/org.eclipse.persistence.jpars
 eclipselink.jpars.test=${eclipselink.jpa.base}/eclipselink.jpars.test

diff --git a/antbuild.xml b/antbuild.xml
@@ -1,6 +1,6 @@
 <!--
 
-    Copyright (c) 1998, 2024 Oracle and/or its affiliates. All rights reserved.
+    Copyright (c) 1998, 2025 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 v. 2.0 which is available at
@@ -65,6 +65,7 @@
         - test-core-srg : runs core SRG
         - test-jpa : runs JPA tests
         - test-jpa-jse : runs JPA JSE tests
+        - test-deadlock : runs Deadlock detection tests
         - test-moxy : runs MOXY tests
         - test-moxy-srg : runs MOXY SRG
         - test-jaxb : runs MOXY jaxb tests
@@ -557,6 +558,10 @@
         <delete dir="${eclipselink.jpa.test.jse}/${classes.dir}" />
         <delete dir="${eclipselink.jpa.test.jse}/${run.dir}" />
         <delete dir="${eclipselink.jpa.test.jse}/${report.dir}" />
+        <!-- Clean Deadlock Test. Ant project can't be called without class org.eclipse.persistence.tools.weaving.jpa.StaticWeaveAntTask on the classpath (custom Ant task) -->
+        <delete dir="${eclipselink.deadlock.test}/${classes.dir}" />
+        <delete dir="${eclipselink.deadlock.test}/${run.dir}" />
+        <delete dir="${eclipselink.deadlock.test}/${report.dir}" />
 
         <ant antfile="antbuild.xml" dir="${eclipselink.jpa.wdf.test}"        target="clean"/>
         <ant antfile="antbuild.xml" dir="${eclipselink.jpars.test}"          target="clean"/>
@@ -1855,7 +1860,7 @@
             depends="clear-db, test-core-srg, test-jpa22-srg, test-jpars, test-moxy-srg, test-sdo-srg, test-dbws-srg, test-dbws-builder-srg, generate-report"
             />
     <target name="test-lrg-only" description="run all the LRG test suites"
-            depends="test-core, test-ext, test-jpa22, test-jpa-jse, test-jpql, test-wdf, test-jpars, test-moxy, test-sdo, test-dbws, test-dbws-builder, test-osgi"
+            depends="test-core, test-ext, test-jpa22, test-jpa-jse, test-deadlock, test-jpql, test-wdf, test-jpars, test-moxy, test-sdo, test-dbws, test-dbws-builder, test-osgi"
             />
     <target name="test-lrg" description="run all the LRG test suites"
             depends="clear-db, test-lrg-only, generate-report"
@@ -1999,6 +2004,10 @@
     <target name="test-jpa-jse" description="run the JPA JSE tests">
         <ant antfile="antbuild.xml" dir="${eclipselink.jpa.test.jse}" target="test"/>
     </target>
+    <!-- run Deadlock detection tests -->
+    <target name="test-deadlock" description="run Deadlock detection tests">
+        <ant antfile="antbuild.xml" dir="${eclipselink.deadlock.test}" target="test"/>
+    </target>
 
     <target name="init-coverage">
         <taskdef uri="antlib:org.jacoco.ant" resource="org/jacoco/ant/antlib.xml">

diff --git a/...ipselink.core.test/src/org/eclipse/persistence/testing/tests/junit/helper/HelperTest.java b/...ipselink.core.test/src/org/eclipse/persistence/testing/tests/junit/helper/HelperTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2025 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 v. 2.0 which is available at
@@ -20,6 +20,8 @@
 import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
 import java.util.Vector;
 
 import org.eclipse.persistence.internal.helper.Helper;
@@ -315,4 +317,22 @@ public void timestampFromStringTest() {
             Helper.setShouldOptimizeDates(optimizedDatesState);
         }
     }
+
+    @Test
+    public void copyMapTest() {
+        Map<Long, String> inputMap = new HashMap<>();
+        inputMap.put(1L, "one");
+        inputMap.put(2L, "two");
+        Map<Long, String> copyMap = Helper.copyMap(inputMap);
+        Assert.assertEquals(inputMap, copyMap);
+    }
+
+    @Test
+    public void copySetTest() {
+        Set<String> inputSet = new HashSet<>();
+        inputSet.add("one");
+        inputSet.add("two");
+        Set<String> copySet = Helper.copySet(inputSet);
+        Assert.assertEquals(inputSet, copySet);
+    }
 }
diff --git a/...clipse.persistence.core/src/org/eclipse/persistence/config/MergeManagerOperationMode.java b/...clipse.persistence.core/src/org/eclipse/persistence/config/MergeManagerOperationMode.java
@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2025 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 v. 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0,
+ * or the Eclipse Distribution License v. 1.0 which is available at
+ * http://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
+ */
+
+// Contributors:
+//     Oracle - initial API and implementation
+package org.eclipse.persistence.config;
+
+/**
+ * INTERNAL:
+ * <p>
+ * <b>Purpose</b>: It is data model behind {@linkplain  org.eclipse.persistence.config.SystemProperties#CONCURRENCY_MANAGER_ALLOW_GET_CACHE_KEY_FOR_MERGE_MODE}
+ * or {@linkplain  org.eclipse.persistence.config.PersistenceUnitProperties#CONCURRENCY_MANAGER_ALLOW_GET_CACHE_KEY_FOR_MERGE_MODE}.
+ */
+public final class MergeManagerOperationMode {
+
+    /**
+     * {@code ORIGIN} (DEFAULT) - There is infinite {@linkplain java.lang.Object#wait()} call in case of some conditions during time when object/entity referred from
+     * {@code org.eclipse.persistence.internal.identitymaps.CacheKey} is locked and modified by another thread. In some cases it should leads into deadlock.
+     */
+    public static final String ORIGIN = "ORIGIN";
+
+    /**
+     * {@code WAITLOOP} - Merge manager will try in the loop with timeout wait {@code cacheKey.wait(ConcurrencyUtil.SINGLETON.getAcquireWaitTime());}
+     * fetch object/entity from {@linkplain org.eclipse.persistence.internal.identitymaps.CacheKey}. If fetch will be successful object/entity loop finish and continue
+     * with remaining code. If not @{code java.lang.InterruptedException} is thrown and caught and used {@linkplain org.eclipse.persistence.internal.identitymaps.CacheKey} instance
+     * status is set into invalidation state. This strategy avoid deadlock issue, but there should be impact to the performance.
+     */
+    public static final String WAITLOOP = "WAITLOOP";
+
+    private MergeManagerOperationMode() {
+        // no instance please
+    }
+}
diff --git a/...clipse.persistence.core/src/org/eclipse/persistence/config/PersistenceUnitProperties.java b/...clipse.persistence.core/src/org/eclipse/persistence/config/PersistenceUnitProperties.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2023 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2025 Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 1998, 2022 IBM Corporation. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -4051,6 +4051,23 @@ public class PersistenceUnitProperties {
      */
     public static final String CONCURRENCY_MANAGER_ALLOW_INTERRUPTED_EXCEPTION  = "eclipselink.concurrency.manager.allow.interruptedexception";
 
+    /**
+     * <p>
+     * This property control in {@link org.eclipse.persistence.internal.sessions.AbstractSession#getCacheKeyFromTargetSessionForMerge(java.lang.Object, org.eclipse.persistence.internal.descriptors.ObjectBuilder, org.eclipse.persistence.descriptors.ClassDescriptor, org.eclipse.persistence.internal.sessions.MergeManager)}
+     * strategy how {@code org.eclipse.persistence.internal.identitymaps.CacheKey} will be fetched from shared cache.
+     * <p>
+     * <b>Allowed Values</b> (case-sensitive String)<b>:</b>
+     * <ul>
+     * <li>{@code ORIGIN} (DEFAULT) - There is infinite {@linkplain java.lang.Object#wait()} call in case of some conditions during time when object/entity referred from
+     * {@code org.eclipse.persistence.internal.identitymaps.CacheKey} is locked and modified by another thread. In some cases it should leads into deadlock.
+     * <li>{@code WAITLOOP} - Merge manager will try in the loop with timeout wait {@code cacheKey.wait(ConcurrencyUtil.SINGLETON.getAcquireWaitTime());}
+     * fetch object/entity from {@linkplain org.eclipse.persistence.internal.identitymaps.CacheKey}. If fetch will be successful object/entity loop finish and continue
+     * with remaining code. If not @{code java.lang.InterruptedException} is thrown and caught and used {@linkplain org.eclipse.persistence.internal.identitymaps.CacheKey} instance
+     * status is set into invalidation state. This strategy avoid deadlock issue, but there should be impact to the performance.
+     * </ul>
+     */
+    public static final String CONCURRENCY_MANAGER_ALLOW_GET_CACHE_KEY_FOR_MERGE_MODE  = "eclipselink.concurrency.manager.allow.getcachekeyformerge.mode";
+
     /**
      * <p>
      * This property control (enable/disable) if <code>ConcurrencyException</code> fired when dead-lock diagnostic is enabled.

diff --git a/...ion/org.eclipse.persistence.core/src/org/eclipse/persistence/config/SystemProperties.java b/...ion/org.eclipse.persistence.core/src/org/eclipse/persistence/config/SystemProperties.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2021 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2025 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 v. 2.0 which is available at
@@ -155,6 +155,23 @@ public class SystemProperties {
      */
     public static final String CONCURRENCY_MANAGER_ALLOW_INTERRUPTED_EXCEPTION  = "eclipselink.concurrency.manager.allow.interruptedexception";
 
+    /**
+     * <p>
+     * This property control in {@link org.eclipse.persistence.internal.sessions.AbstractSession#getCacheKeyFromTargetSessionForMerge(java.lang.Object, org.eclipse.persistence.internal.descriptors.ObjectBuilder, org.eclipse.persistence.descriptors.ClassDescriptor, org.eclipse.persistence.internal.sessions.MergeManager)}
+     * strategy how {@code org.eclipse.persistence.internal.identitymaps.CacheKey} will be fetched from shared cache.
+     * <p>
+     * <b>Allowed Values</b> (case-sensitive String)<b>:</b>
+     * <ul>
+     * <li>{@code ORIGIN} (DEFAULT) - There is infinite {@linkplain java.lang.Object#wait()} call in case of some conditions during time when object/entity referred from
+     * {@code org.eclipse.persistence.internal.identitymaps.CacheKey} is locked and modified by another thread. In some cases it should leads into deadlock.
+     * <li>{@code WAITLOOP} - Merge manager will try in the loop with timeout wait {@code cacheKey.wait(ConcurrencyUtil.SINGLETON.getAcquireWaitTime());}
+     * fetch object/entity from {@linkplain org.eclipse.persistence.internal.identitymaps.CacheKey}. If fetch will be successful object/entity loop finish and continue
+     * with remaining code. If not @{code java.lang.InterruptedException} is thrown and caught and used {@linkplain org.eclipse.persistence.internal.identitymaps.CacheKey} instance
+     * status is set into invalidation state. This strategy avoid deadlock issue, but there should be impact to the performance.
+     * </ul>
+     */
+    public static final String CONCURRENCY_MANAGER_ALLOW_GET_CACHE_KEY_FOR_MERGE_MODE  = "eclipselink.concurrency.manager.allow.getcachekeyformerge.mode";
+
     /**
      * <p>
      * This property control (enable/disable) if <code>ConcurrencyException</code> fired when dead-lock diagnostic is enabled.

diff --git a/...ipse.persistence.core/src/org/eclipse/persistence/internal/helper/ConcurrencyManager.java b/...ipse.persistence.core/src/org/eclipse/persistence/internal/helper/ConcurrencyManager.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2024 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2025 Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2021 IBM Corporation. All rights reserved.
  *
  * This program and the accompanying materials are made available under the
@@ -429,6 +429,13 @@ protected static Map<Thread, DeferredLockManager> getDeferredLockManagers() {
         return DEFERRED_LOCK_MANAGERS;
     }
 
+    /**
+     * Return snapshot of the deferred lock manager hashtable (thread - DeferredLockManager).
+     */
+    protected static Map<Thread, DeferredLockManager> getDeferredLockManagersSnapshot() {
+        return Helper.copyMap(DEFERRED_LOCK_MANAGERS);
+    }
+
     /**
      * Init the deferred lock managers (thread - DeferredLockManager).
      */
@@ -853,7 +860,7 @@ public void releaseAllLocksAcquiredByThread(DeferredLockManager lockManager) {
      * @return Never null if the read lock manager does not yet exist for the current thread. otherwise its read log
      *         manager is returned.
      */
-    protected static ReadLockManager getReadLockManager(Thread thread) {
+    public static ReadLockManager getReadLockManager(Thread thread) {
         Map<Thread, ReadLockManager> readLockManagers = getReadLockManagers();
         return readLockManagers.get(thread);
     }
@@ -865,6 +872,13 @@ protected static Map<Thread, ReadLockManager> getReadLockManagers() {
         return READ_LOCK_MANAGERS;
     }
 
+    /**
+     * Return snapshot of the deferred lock manager hashtable (thread - DeferredLockManager).
+     */
+    protected static Map<Thread, ReadLockManager> getReadLockManagersSnapshot() {
+        return Helper.copyMap(READ_LOCK_MANAGERS);
+    }
+
     /**
      * Print the nested depth.
      */
@@ -969,33 +983,33 @@ public long getTotalNumberOfKeysReleasedForReadingBlewUpExceptionDueToCacheKeyHa
     }
 
     /** Getter for {@link #THREADS_TO_WAIT_ON_ACQUIRE} */
-    public static Map<Thread, ConcurrencyManager> getThreadsToWaitOnAcquire() {
-        return new HashMap<>(THREADS_TO_WAIT_ON_ACQUIRE);
+    public static Map<Thread, ConcurrencyManager> getThreadsToWaitOnAcquireSnapshot() {
+        return Helper.copyMap(THREADS_TO_WAIT_ON_ACQUIRE);
     }
 
     /** Getter for {@link #THREADS_TO_WAIT_ON_ACQUIRE_NAME_OF_METHOD_CREATING_TRACE} */
-    public static Map<Thread, String> getThreadsToWaitOnAcquireMethodName() {
-        return new HashMap<>(THREADS_TO_WAIT_ON_ACQUIRE_NAME_OF_METHOD_CREATING_TRACE);
+    public static Map<Thread, String> getThreadsToWaitOnAcquireMethodNameSnapshot() {
+        return Helper.copyMap(THREADS_TO_WAIT_ON_ACQUIRE_NAME_OF_METHOD_CREATING_TRACE);
     }
 
     /** Getter for {@link #THREADS_TO_WAIT_ON_ACQUIRE_READ_LOCK} */
-    public static Map<Thread, ConcurrencyManager> getThreadsToWaitOnAcquireReadLock() {
-        return THREADS_TO_WAIT_ON_ACQUIRE_READ_LOCK;
+    public static Map<Thread, ConcurrencyManager> getThreadsToWaitOnAcquireReadLockSnapshot() {
+        return Helper.copyMap(THREADS_TO_WAIT_ON_ACQUIRE_READ_LOCK);
     }
 
     /** Getter for {@link #THREADS_TO_WAIT_ON_ACQUIRE_READ_LOCK_NAME_OF_METHOD_CREATING_TRACE} */
-    public static Map<Thread, String> getThreadsToWaitOnAcquireReadLockMethodName() {
-        return THREADS_TO_WAIT_ON_ACQUIRE_READ_LOCK_NAME_OF_METHOD_CREATING_TRACE;
+    public static Map<Thread, String> getThreadsToWaitOnAcquireReadLockMethodNameSnapshot() {
+        return Helper.copyMap(THREADS_TO_WAIT_ON_ACQUIRE_READ_LOCK_NAME_OF_METHOD_CREATING_TRACE);
     }
 
     /** Getter for {@link #THREADS_WAITING_TO_RELEASE_DEFERRED_LOCKS} */
-    public static Set<Thread> getThreadsWaitingToReleaseDeferredLocks() {
-        return new HashSet<>(THREADS_WAITING_TO_RELEASE_DEFERRED_LOCKS);
+    public static Set<Thread> getThreadsWaitingToReleaseDeferredLocksSnapshot() {
+        return Helper.copySet(THREADS_WAITING_TO_RELEASE_DEFERRED_LOCKS);
     }
 
     /** Getter for {@link #THREADS_WAITING_TO_RELEASE_DEFERRED_LOCKS_BUILD_OBJECT_COMPLETE_GOES_NOWHERE} */
-    public static Map<Thread, String> getThreadsWaitingToReleaseDeferredLocksJustification() {
-        return new HashMap<>(THREADS_WAITING_TO_RELEASE_DEFERRED_LOCKS_BUILD_OBJECT_COMPLETE_GOES_NOWHERE);
+    public static Map<Thread, String> getThreadsWaitingToReleaseDeferredLocksJustificationSnapshot() {
+        return Helper.copyMap(THREADS_WAITING_TO_RELEASE_DEFERRED_LOCKS_BUILD_OBJECT_COMPLETE_GOES_NOWHERE);
     }
 
     /**
@@ -1107,4 +1121,32 @@ public Lock getInstanceLock() {
     public Condition getInstanceLockCondition() {
         return this.instanceLockCondition;
     }
+
+    public boolean isCacheKey() {
+        return false;
+    }
+
+    /**
+     * Check if {@code org.eclipse.persistence.internal.helper.ConcurrencyManager} or child like {@code org.eclipse.persistence.internal.identitymaps.CacheKey} is currently being owned for writing
+     * and if that owning thread happens to be the current thread doing the check.
+     *
+     * @return {@code false} means either the thread is currently not owned by any other thread for writing purposes. Or otherwise if is owned by some thread
+     * but the thread is not the current thread. {@code false} is returned if and only if instance is being owned by some thread
+     * and that thread is not the current thread, it is some other competing thread.
+     */
+    public boolean isAcquiredForWritingAndOwnedByDifferentThread() {
+        instanceLock.lock();
+        try {
+            if (!this.isAcquired()) {
+                return false;
+            }
+            if (this.activeThread == null) {
+                return false;
+            }
+            Thread currentThread = Thread.currentThread();
+            return this.activeThread != currentThread;
+        } finally {
+            instanceLock.unlock();
+        }
+    }
 }