Eclipselink 2.7.x Tracing metadata - bug in method org.eclipse.persistence.internal.helper.ConcurrencyManager.putThreadAsWaitingToAcquireLockForWriting(Thread, String) - We end up with metadata on threads wanting to acquire write locks

[99sono]

Eclipselink 2.7.x Tracing metadata - bug in method org.eclipse.persistence.internal.helper.ConcurrencyManager.putThreadAsWaitingToAcquireLockForWriting(Thread, String) - We end up with metadata on threads wanting to acquire write locks

[99sono]

This issue is now documented under a service request. Thanks a lot.

[99sono]

Side note: The 2.7.15 tag appears to address a defect related to the ConcurrencyManager. You can find the relevant changes in the following commits:

1. Update ConcurrencyManager implementation to be in sync with master
2. Pull request #1347

Additionally, here's the direct link to the specific commit:
Commit details

3. The code in the Write Lock manager looks to have been modified:
   [2.7.15 write lock manager with modified acquireLocksForClone] (https://github.com/eclipse-ee4j/eclipselink/blob/2.7.15/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/helper/WriteLockManager.java)

I am some hard time following the changes on the method.
the changes look bigger than i anticipated.
I see that the method continues to do a:

putThreadAsWaitingToAcquireLockForWriting

Whereas I would have expected a

lastCacheKeyWeNeededToWaitToAcquire.putCurrentThreadAsWaitingToAcquireLockForReading

As I had explained in point 7.

Also the new piece of code doing:

 toWaitOnLock.lock()

And later

 toWaitOnLock.unlock();

Looks like a radical change to me in the method implementation.

I cannot judge if that code is actually correct or not.

All I can see I was expecting the method implementation to look more like this:

 public Map acquireLocksForClone(Object objectForClone, ClassDescriptor descriptor, CacheKey cacheKey, AbstractSession cloningSession) {
        // TRK-19750 - basic variable initialization to be able to do the
        // determineIfReleaseDeferredLockAppearsToBeDeadLocked
        final Date whileStartDate = new Date();
        final Thread currentThread = Thread.currentThread();
        DeferredLockManager lockManager = ConcurrencyManager.getDeferredLockManager(currentThread);
        HackingEclipseReadLockManager readLockManager = ConcurrencyManager.getReadLockManager(currentThread);

        boolean successful = false;
        IdentityHashMap lockedObjects = new IdentityHashMap();
        IdentityHashMap refreshedObjects = new IdentityHashMap();

        CacheKey lastCacheKeyWeNeededToWaitToAcquire = null;
        try {
            // if the descriptor has indirection for all mappings then wait as there will be no deadlock risks
            CacheKey toWaitOn = acquireLockAndRelatedLocks(objectForClone, lockedObjects, refreshedObjects, cacheKey, descriptor, cloningSession);
            int tries = 0;
            while (toWaitOn != null) {// loop until we've tried too many times.

                for (Iterator lockedList = lockedObjects.values().iterator(); lockedList.hasNext();) {
                    ((CacheKey)lockedList.next()).releaseReadLock();
                    lockedList.remove();
                }

                // TRK-19750 - populate the static hash map
                // of the concurrenyc manager that we use for creating the massive log dump
                // to indicate that the current thread is now stuck trying to acquire some arbitrary
                // cache key for writing
                lastCacheKeyWeNeededToWaitToAcquire = toWaitOn;
                lastCacheKeyWeNeededToWaitToAcquire.putCurrentThreadAsWaitingToAcquireLockForReading(
                        "org.eclipse.persistence.internal.helper.WriteLockManager.acquireLocksForClone(Object, ClassDescriptor, CacheKey, AbstractSession)");

                // TRK-19750 - Since we know this one of those methods that can appear in the dead locks
                // we threads frozen here forever inside of the wait that used to have no timeout
                // we will now always check for how long the current thread is stuck in this while loop going nowhere
                // using the exact same approach we have been adding to the concurrency manager
                HackingEclipseHelperUtil.SINGLETON.determineIfReleaseDeferredLockAppearsToBeDeadLocked(toWaitOn,
                        whileStartDate, lockManager, readLockManager,
                        WRITE_LOCK_MANAGER_IS_WILLING_TO_ALLOW_INTERRUPTED_EXCEPTION_TO_BE_FIRED_UP_IF_CONFIGURATION_WOULD_ALLOW_ID_TRUE);

                synchronized (toWaitOn) {
                    try {
                        if (toWaitOn.isAcquired()) {//last minute check to insure it is still locked.
                            // TRK-19750
                            // this is the famous WriteLockManager.acquireLocksForClone(WriteLockManager.java:92)
                            // being one of the spots where threads trying build objects can get stuck forever
                            // commenting out wait without a timeout
                            // if the thread that has the lock with write permissions is in a dead lock
                            // then we are not coming out
                            // toWaitOn.wait();// wait for lock on object to be released

                            // TRK-19750
                            // at wait with timout like everywhere else waiting without timeout is always wrong
                            toWaitOn.wait(10000l);
                        }
                    } catch (InterruptedException ex) {
                        // Ignore exception thread should continue.
                    }
                }
                Object waitObject = toWaitOn.getObject();
                // Object may be null for loss of identity.
                if (waitObject != null) {
                    cloningSession.checkAndRefreshInvalidObject(waitObject, toWaitOn, cloningSession.getDescriptor(waitObject));
                    refreshedObjects.put(waitObject, waitObject);
                }
                toWaitOn = acquireLockAndRelatedLocks(objectForClone, lockedObjects, refreshedObjects, cacheKey, descriptor, cloningSession);
                if ((toWaitOn != null) && ((++tries) > MAXTRIES)) {
                    // If we've tried too many times abort.
                    throw ConcurrencyException.maxTriesLockOnCloneExceded(objectForClone);
                }
            }
            successful = true;//successfully acquired all locks
        } catch (InterruptedException exception) {
            // TRK-19750 - if determineIfReleaseDeferredLockAppearsToBeDeadLocked is blowing up a thread stuck for too
            // long
            // run the lock of freeing up locks acquired by the thread
            // NOTE: we would be tempted to do this commented code bellow
            // cacheKey.releaseAllLocksAquiredByThread(lockManager, readLockManager);
            // throw ConcurrencyException.waitFailureOnClientSession(exception);
            // Instead what we do is just mimic the vanila behavior we have for the interrupted exception inside of the
            // wait
            // we must assume this is correct behavior
            throw ConcurrencyException.maxTriesLockOnCloneExceded(objectForClone);
        } finally {
            // TRK-19750 - remove from the static hash map
            // of the concurrency manager that we use for creating the massive log dump
            // any information we may have added that this thread was strugling to acquire any particular
            // cache key. The current thread is out of the wait to acquire logic now so we can consider the thread
            // as not no longer being stuck
            if (lastCacheKeyWeNeededToWaitToAcquire != null) {
                lastCacheKeyWeNeededToWaitToAcquire.removeCurrentThreadNoLongerWaitingToAcquireLockForReading();
            }

            if (!successful) {//did not acquire locks but we are exiting
                for (Iterator lockedList = lockedObjects.values().iterator(); lockedList.hasNext();) {
                    ((CacheKey)lockedList.next()).releaseReadLock();
                    lockedList.remove();
                }
            }
        }
        return lockedObjects;
    }

I would assume the changes in the:
https://github.com/eclipse-ee4j/eclipselink/blob/2.7.15/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/helper/WriteLockManager.java

Are correct but I have some hard time seeing that is the case.

[rfelcman]

Sorry but is it this issue still valid? Because methods
public void putThreadAsWaitingToAcquireLockForWriting(Thread thread, String methodName)
https://github.com/eclipse-ee4j/eclipselink/blob/2.7/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/helper/ConcurrencyManager.java#L915

and

public void removeThreadNoLongerWaitingToAcquireLockForWriting(Thread thread)
https://github.com/eclipse-ee4j/eclipselink/blob/2.7/foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/helper/ConcurrencyManager.java#L924

looks OK to me.