Skip to content

Commit

Permalink
Deadlock in AbstractSession implementation - new feature plus unit test
Browse files Browse the repository at this point in the history 
Signed-off-by: Radek Felcman <[email protected]>
  • Loading branch information
@rfelcman
rfelcman committed Jan 23, 2025
1 parent f2eb9b5 commit ceff09d
Show file tree
Hide file tree
Showing 14 changed files with 572 additions and 57 deletions.
53 changes: 52 additions & 1 deletion docs/docs.jpa/src/main/asciidoc/persistenceproperties_ref.adoc
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
///////////////////////////////////////////////////////////////////////////////

Copyright (c) 2022, 2023 Oracle and/or its affiliates. All rights reserved.
Copyright (c) 2022, 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
Expand Down Expand Up @@ -213,6 +213,7 @@ EclipseLink includes the following persistence property extensions for
concurrency management:
* link:#concurrency-manager-allow-concurrencyexception[`concurrency.manager.allow.concurrencyexception`]
* link:#concurrency-manager-allow-getcachekeyformerge-mode[`concurrency.manager.allow.getcachekeyformerge.mode`]
* link:#concurrency-manager-allow-interruptedexception[`concurrency.manager.allow.interruptedexception`]
* link:#concurrency-manager-allow-readlockstacktrace[`concurrency.manager.allow.readlockstacktrace`]
* link:#concurrency-manager-build-object-complete-waittime[`concurrency.manager.build.object.complete.waittime`]
Expand Down Expand Up @@ -286,6 +287,7 @@ The following lists the EclipseLink persistence property
* link:#compositeunitmember[`composite-unit.member`]
* link:#compositeunitproperties[`composite-unit.properties`]
* link:#concurrency-manager-allow-concurrencyexception[`concurrency.manager.allow.concurrencyexception`]
* link:#concurrency-manager-allow-getcachekeyformerge-mode[`concurrency.manager.allow.getcachekeyformerge.mode`]
* link:#concurrency-manager-allow-interruptedexception[`concurrency.manager.allow.interruptedexception`]
* link:#concurrency-manager-allow-readlockstacktrace[`concurrency.manager.allow.readlockstacktrace`]
* link:#concurrency-manager-build-object-complete-waittime[`concurrency.manager.build.object.complete.waittime`]
Expand Down Expand Up @@ -3103,6 +3105,55 @@ persistence.xml_*
'''''
=== concurrency.manager.allow.getcachekeyformerge.mode
This property control in `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 `org.eclipse.persistence.internal.identitymaps.CacheKey` will be fetched from shared cache.
*Values*
link:#concurrency.manager.allow.getcachekeyformerge.mode[Table 5-30]
describes this persistence property's values.
[[concurrency.manager.allow.getcachekeyformerge.mode]]
*_Table 5-30 Valid Values for
concurrency.manager.allow.getcachekeyformerge.mode_*
|=======================================================================
|*Value* |*Description*
|ORIGIN |(Default) There is infinite `java.lang.Object.wait()` call in case
of some conditions during time when object/entity referred from
`org.eclipse.persistence.internal.identitymaps.CacheKey` is locked
and modified by another thread. In some cases it should leads into deadlock.
|WAITLOOP |Merge manager will try in the loop with timeout wait `cacheKey.wait(ConcurrencyUtil.SINGLETON.getAcquireWaitTime());`
fetch object/entity from `org.eclipse.persistence.internal.identitymaps.CacheKey`.
If fetch will be successful object/entity loop finish and continue with remaining code.
If not `java.lang.InterruptedException` is thrown and caught and used `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.
|=======================================================================
*Examples*
link:#concurrency.manager.allow.getcachekeyformerge.mode[Example
5-24] shows how to use this property in the `persistence.xml` file.
[[concurrency.manager.allow.getcachekeyformerge.mode]]
*_Example 5-24 Using concurrency.manager.allow.getcachekeyformerge.mode in
persistence.xml_*
[source,oac_no_warn]
----
<property name="eclipselink.concurrency.manager.allow.getcachekeyformerge.mode" value="WAITLOOP" />
----
'''''
=== concurrency.manager.allow.concurrencyexception
See valid values table.
Expand Down
47 changes: 32 additions & 15 deletions ...sistence.core/src/main/java/org/eclipse/persistence/config/PersistenceUnitProperties.java
View file
Original file line number Diff line number Diff line change
@@ -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) 1998, 2024 IBM Corporation. All rights reserved.
*
* This program and the accompanying materials are made available under the
Expand Down Expand Up @@ -4056,36 +4056,36 @@ public final class PersistenceUnitProperties {
* mechanism and related extended logging will be activated.
* Default value is 0 (unit is ms). Allowed values are: long
*/
public static final String CONCURRENCY_MANAGER_ACQUIRE_WAIT_TIME = "eclipselink.concurrency.manager.waittime";
public static final String CONCURRENCY_MANAGER_ACQUIRE_WAIT_TIME = SystemProperties.CONCURRENCY_MANAGER_ACQUIRE_WAIT_TIME;

/**
* This system property in milliseconds can control thread management in org.eclipse.persistence.internal.helper.ConcurrencyManager.
* It controls how much time ConcurrencyManager will wait before it will identify, that thread which builds new object/entity instance
* should be identified as a potential deadlock source. It leads into some additional log messages.
* Default value is 0 (unit is ms). In this case extended logging is not active. Allowed values are: long
*/
public static final String CONCURRENCY_MANAGER_BUILD_OBJECT_COMPLETE_WAIT_TIME = "eclipselink.concurrency.manager.build.object.complete.waittime";
public static final String CONCURRENCY_MANAGER_BUILD_OBJECT_COMPLETE_WAIT_TIME = SystemProperties.CONCURRENCY_MANAGER_BUILD_OBJECT_COMPLETE_WAIT_TIME;

/**
* This system property in milliseconds can control thread management in org.eclipse.persistence.internal.helper.ConcurrencyManager.
* It controls how long we are willing to wait before firing up an exception
* Default value is 40000 (unit is ms). Allowed values are: long
*/
public static final String CONCURRENCY_MANAGER_MAX_SLEEP_TIME = "eclipselink.concurrency.manager.maxsleeptime";
public static final String CONCURRENCY_MANAGER_MAX_SLEEP_TIME = SystemProperties.CONCURRENCY_MANAGER_MAX_SLEEP_TIME;

/**
* This system property in milliseconds can control thread management in org.eclipse.persistence.internal.helper.ConcurrencyManager and org.eclipse.persistence.internal.helper.ConcurrencyUtil.
* It controls how frequently the tiny dump log message is created.
* Default value is 40000 (unit is ms). Allowed values are: long
*/
public static final String CONCURRENCY_MANAGER_MAX_FREQUENCY_DUMP_TINY_MESSAGE = "eclipselink.concurrency.manager.maxfrequencytodumptinymessage";
public static final String CONCURRENCY_MANAGER_MAX_FREQUENCY_DUMP_TINY_MESSAGE = SystemProperties.CONCURRENCY_MANAGER_MAX_FREQUENCY_DUMP_TINY_MESSAGE;

/**
* This system property in milliseconds can control thread management in org.eclipse.persistence.internal.helper.ConcurrencyManager and org.eclipse.persistence.internal.helper.ConcurrencyUtil.
* It controls how frequently the massive dump log message is created.
* Default value is 60000 (unit is ms). Allowed values are: long
*/
public static final String CONCURRENCY_MANAGER_MAX_FREQUENCY_DUMP_MASSIVE_MESSAGE = "eclipselink.concurrency.manager.maxfrequencytodumpmassivemessage";
public static final String CONCURRENCY_MANAGER_MAX_FREQUENCY_DUMP_MASSIVE_MESSAGE = SystemProperties.CONCURRENCY_MANAGER_MAX_FREQUENCY_DUMP_MASSIVE_MESSAGE;

/**
* <p>
Expand All @@ -4103,7 +4103,24 @@ public final class PersistenceUnitProperties {
* threads to progress.
* </ul>
*/
public static final String CONCURRENCY_MANAGER_ALLOW_INTERRUPTED_EXCEPTION = "eclipselink.concurrency.manager.allow.interruptedexception";
public static final String CONCURRENCY_MANAGER_ALLOW_INTERRUPTED_EXCEPTION = SystemProperties.CONCURRENCY_MANAGER_ALLOW_INTERRUPTED_EXCEPTION;

/**
* <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 {@code 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 {@code 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 {@code 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>
Expand All @@ -4120,7 +4137,7 @@ public final class PersistenceUnitProperties {
* locks and allow other threads to progress.
* </ul>
*/
public static final String CONCURRENCY_MANAGER_ALLOW_CONCURRENCY_EXCEPTION = "eclipselink.concurrency.manager.allow.concurrencyexception";
public static final String CONCURRENCY_MANAGER_ALLOW_CONCURRENCY_EXCEPTION = SystemProperties.CONCURRENCY_MANAGER_ALLOW_CONCURRENCY_EXCEPTION;

/**
* <p>
Expand All @@ -4132,7 +4149,7 @@ public final class PersistenceUnitProperties {
* <li>{@code true} - collect debug/trace information during ReadLock acquisition. Has negative impact to the performance.
* </ul>
*/
public static final String CONCURRENCY_MANAGER_ALLOW_STACK_TRACE_READ_LOCK = "eclipselink.concurrency.manager.allow.readlockstacktrace";
public static final String CONCURRENCY_MANAGER_ALLOW_STACK_TRACE_READ_LOCK = SystemProperties.CONCURRENCY_MANAGER_ALLOW_STACK_TRACE_READ_LOCK;

/**
* <p>
Expand All @@ -4150,7 +4167,7 @@ public final class PersistenceUnitProperties {
* vanilla behavior).
* </ul>
*/
public static final String CONCURRENCY_MANAGER_USE_SEMAPHORE_TO_SLOW_DOWN_OBJECT_BUILDING = "eclipselink.concurrency.manager.object.building.semaphore";
public static final String CONCURRENCY_MANAGER_USE_SEMAPHORE_TO_SLOW_DOWN_OBJECT_BUILDING = SystemProperties.CONCURRENCY_MANAGER_USE_SEMAPHORE_TO_SLOW_DOWN_OBJECT_BUILDING;

/**
* <p>
Expand All @@ -4169,7 +4186,7 @@ public final class PersistenceUnitProperties {
* vanilla behavior).
* </ul>
*/
public static final String CONCURRENCY_MANAGER_USE_SEMAPHORE_TO_SLOW_DOWN_WRITE_LOCK_MANAGER_ACQUIRE_REQUIRED_LOCKS = "eclipselink.concurrency.manager.write.lock.manager.semaphore";
public static final String CONCURRENCY_MANAGER_USE_SEMAPHORE_TO_SLOW_DOWN_WRITE_LOCK_MANAGER_ACQUIRE_REQUIRED_LOCKS = SystemProperties.CONCURRENCY_MANAGER_USE_SEMAPHORE_TO_SLOW_DOWN_WRITE_LOCK_MANAGER_ACQUIRE_REQUIRED_LOCKS;

/**
* <p>
Expand All @@ -4178,7 +4195,7 @@ public final class PersistenceUnitProperties {
* If "eclipselink.concurrency.manager.object.building.semaphore" property is {@code false} (DEFAULT) number of threads is unlimited.
* </p>
*/
public static final String CONCURRENCY_MANAGER_OBJECT_BUILDING_NO_THREADS = "eclipselink.concurrency.manager.object.building.no.threads";
public static final String CONCURRENCY_MANAGER_OBJECT_BUILDING_NO_THREADS = SystemProperties.CONCURRENCY_MANAGER_OBJECT_BUILDING_NO_THREADS;

/**
* <p>
Expand All @@ -4187,7 +4204,7 @@ public final class PersistenceUnitProperties {
* If "eclipselink.concurrency.manager.write.lock.manager.semaphore" property is {@code false} (DEFAULT) number of threads is unlimited.
* </p>
*/
public static final String CONCURRENCY_MANAGER_WRITE_LOCK_MANAGER_ACQUIRE_REQUIRED_LOCKS_NO_THREADS = "eclipselink.concurrency.manager.write.lock.manager.no.threads";
public static final String CONCURRENCY_MANAGER_WRITE_LOCK_MANAGER_ACQUIRE_REQUIRED_LOCKS_NO_THREADS = SystemProperties.CONCURRENCY_MANAGER_WRITE_LOCK_MANAGER_ACQUIRE_REQUIRED_LOCKS_NO_THREADS;

/**
* <p>
Expand All @@ -4196,15 +4213,15 @@ public final class PersistenceUnitProperties {
* Default value is 2000 (unit is ms). Allowed values are: long
* </p>
*/
public static final String CONCURRENCY_SEMAPHORE_MAX_TIME_PERMIT = "eclipselink.concurrency.semaphore.max.time.permit";
public static final String CONCURRENCY_SEMAPHORE_MAX_TIME_PERMIT = SystemProperties.CONCURRENCY_SEMAPHORE_MAX_TIME_PERMIT;

/**
* <p>
* This property control timeout between log messages in {@link org.eclipse.persistence.internal.helper.ConcurrencySemaphore#acquireSemaphoreIfAppropriate(boolean)} when method/thread tries to get permit for the execution.
* Default value is 10000 (unit is ms). Allowed values are: long
* </p>
*/
public static final String CONCURRENCY_SEMAPHORE_LOG_TIMEOUT = "eclipselink.concurrency.semaphore.log.timeout";
public static final String CONCURRENCY_SEMAPHORE_LOG_TIMEOUT = SystemProperties.CONCURRENCY_SEMAPHORE_LOG_TIMEOUT;

/**
* <p>
Expand Down
21 changes: 19 additions & 2 deletions ...lipse.persistence.core/src/main/java/org/eclipse/persistence/config/SystemProperties.java
View file
Original file line number Diff line number Diff line change
@@ -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) 2024 Contributors to the Eclipse Foundation. All rights reserved.
*
* This program and the accompanying materials are made available under the
Expand Down Expand Up @@ -158,6 +158,23 @@ public final 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 {@code 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 {@code 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 {@code 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} fired when dead-lock diagnostic is enabled.
Expand All @@ -173,7 +190,7 @@ public final class SystemProperties {
* locks and allow other threads to progress.
* </ul>
*/
public static final String CONCURRENCY_MANAGER_ALLOW_CONCURRENCY_EXCEPTION = "eclipselink.concurrency.manager.allow.concurrency.exception";
public static final String CONCURRENCY_MANAGER_ALLOW_CONCURRENCY_EXCEPTION = "eclipselink.concurrency.manager.allow.concurrencyexception";

/**
* <p>
Expand Down
28 changes: 26 additions & 2 deletions ...stence.core/src/main/java/org/eclipse/persistence/internal/helper/ConcurrencyManager.java
View file
Original file line number Diff line number Diff line change
@@ -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.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
Expand Down Expand Up @@ -863,7 +863,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);
}
Expand Down Expand Up @@ -1121,4 +1121,28 @@ public Lock getInstanceLock() {
public Condition getInstanceLockCondition() {
return this.instanceLockCondition;
}

/**
* 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();
}
}
}
Loading

0 comments on commit ceff09d

Please sign in to comment.