From f6dca86546d7cd414b8f310d8a8ad48fd2dc02fd Mon Sep 17 00:00:00 2001 From: 99sono Date: Mon, 29 Apr 2024 09:36:49 +0200 Subject: [PATCH] TRK-32315: Addressing Concurrency Issues in ReadLockManager Continuing our efforts to resolve issue https://github.com/eclipse-ee4j/eclipselink/issues/2114, we have identified the root cause of the bug. The crux of the matter lies in the `ReadLockManager` providing the `ConcurrencyUtil` access to its state without performing deep copies. Specifically, the `ReadLockManager` exposed its state through unmodifiable hash maps and lists. However, beneath these unmodifiable wrappers, the true data structures of the class remained accessible. During the process of generating massive dumps, the `ConcurrencyUtil` iterates over metadata. Simultaneously, the read lock manager's data structures could undergo additions and removals of cache keys. These lists behind the scenes are not meant to be thread-safe since they are always acessed behind synchronized methods, except when the concurrency util itself would iterate over them. This would lead out our mysterious NULL entries in the lists. Fundamentally, the code itself did not logically allow NULL values to be entered into the read lock manager. However, the concurrency between the read lock manager's internal logic and the massive dump generation could inadvertently corrupt the data structure, resulting in this phenomenon. --- .../config/PersistenceUnitProperties.java | 6 +- .../internal/helper/ConcurrencyUtil.java | 38 +++- .../internal/helper/ReadLockManager.java | 135 ++++++++++-- .../type/ReadLockAcquisitionMetadataTest.java | 195 ++++++++++++++++++ 4 files changed, 353 insertions(+), 21 deletions(-) create mode 100644 src/test/java/org/eclipse/persistence/internal/helper/type/ReadLockAcquisitionMetadataTest.java diff --git a/src/main/java/org/eclipse/persistence/config/PersistenceUnitProperties.java b/src/main/java/org/eclipse/persistence/config/PersistenceUnitProperties.java index b3e0961..79c907b 100644 --- a/src/main/java/org/eclipse/persistence/config/PersistenceUnitProperties.java +++ b/src/main/java/org/eclipse/persistence/config/PersistenceUnitProperties.java @@ -2666,8 +2666,8 @@ public class PersistenceUnitProperties { /** * The "eclipselink.allow-null-max-min" property configures if zero - * is considered a valid return value for MAX/MIN aggregate functions. - * + * is considered a valid return value for MAX/MIN aggregate functions. + * * Section 4.8.5 of the JPA specification dictates this property must default 'true'. * * Default: "true". @@ -3809,7 +3809,7 @@ public class PersistenceUnitProperties { * An NoSQL datasource is a non-relational datasource such as a legacy database, NoSQL database, * XML database, transactional and messaging systems, or ERP systems. * - * @see javax.resource.cci.ConnectionFactory + * see javax.resource.cci.ConnectionFactory */ public static final String NOSQL_CONNECTION_FACTORY = "eclipselink.nosql.connection-factory"; diff --git a/src/main/java/org/eclipse/persistence/internal/helper/ConcurrencyUtil.java b/src/main/java/org/eclipse/persistence/internal/helper/ConcurrencyUtil.java index 6e85692..15a2088 100644 --- a/src/main/java/org/eclipse/persistence/internal/helper/ConcurrencyUtil.java +++ b/src/main/java/org/eclipse/persistence/internal/helper/ConcurrencyUtil.java @@ -887,11 +887,39 @@ public class ConcurrencyUtil { int readLockNumber = 0; for (ReadLockAcquisitionMetadata currentReadLockAcquiredAndNeverReleased : readLocksAcquiredByThread) { readLockNumber++; - writer.write(TraceLocalization.buildMessage("concurrency_util_summary_read_locks_on_thread_step002_3", new Object[] {readLockNumber, - SINGLETON.createToStringExplainingOwnedCacheKey(currentReadLockAcquiredAndNeverReleased.getCacheKeyWhoseNumberOfReadersThreadIsIncrementing()), - ConversionManager.getDefaultManager().convertObject(currentReadLockAcquiredAndNeverReleased.getDateOfReadLockAcquisition(), String.class).toString(), - currentReadLockAcquiredAndNeverReleased.getNumberOfReadersOnCacheKeyBeforeIncrementingByOne(), - currentReadLockAcquiredAndNeverReleased.getCurrentThreadStackTraceInformationCpuTimeCostMs()})); + if(currentReadLockAcquiredAndNeverReleased == null) { + // Note: This line of code should never be executed in real life + // however if we look at the issue: + // TRK-32315: https://github.com/eclipse-ee4j/eclipselink/issues/2114 + // We can see a null pointer exception taking place at + // at org.eclipse.persistence.internal.helper.ConcurrencyUtil.createStringWithSummaryOfReadLocksAcquiredByThread(ConcurrencyUtil.java:891) ~[eclipselink_2_7_6_hacked.jar] + // This was almost certainly the case because the mehod readLockManager.getMapThreadToReadLockAcquisitionMetadata() + // in the past was not returning a deep copy of the map, hence while producing a massive dump + // another thread might be acquiring / releasing read locks and be manipulating the state we are looping over here + // this is no longer the case since the createStringWithSummaryOfReadLocksAcquiredByThread now returns a deep copy + // in any case we add this piece of paranoia code to reduce the probablity of issue as much as possible + // Note 2: We have also further strengthened the code the ReadLockManager itself. + // We have added more sanity checks into that code. Most likely the blow ups of NPException the read lock manager + // were the result of this looping over lists that were not thread safe + + continue; + } + + if(currentReadLockAcquiredAndNeverReleased == null) { + writer.write(TraceLocalization.buildMessage("concurrency_util_summary_read_locks_on_thread_step002_3", new Object[] { + // param 00 + readLockNumber, + // param 01 + SINGLETON.createToStringExplainingOwnedCacheKey(currentReadLockAcquiredAndNeverReleased.getCacheKeyWhoseNumberOfReadersThreadIsIncrementing()), + // param 02 + ConversionManager.getDefaultManager().convertObject(currentReadLockAcquiredAndNeverReleased.getDateOfReadLockAcquisition(), String.class).toString(), + // param 03 + currentReadLockAcquiredAndNeverReleased.getNumberOfReadersOnCacheKeyBeforeIncrementingByOne(), + // param 04 + currentReadLockAcquiredAndNeverReleased.getCurrentThreadStackTraceInformationCpuTimeCostMs() + // finish write block. + })); + } String stackTraceInformation = currentReadLockAcquiredAndNeverReleased.getCurrentThreadStackTraceInformation(); if (stackTraceStringToStackTraceExampleNumber.containsKey(stackTraceInformation)) { // we can spare the massive dump from being any fatter we have alreayd added a stack trace id that diff --git a/src/main/java/org/eclipse/persistence/internal/helper/ReadLockManager.java b/src/main/java/org/eclipse/persistence/internal/helper/ReadLockManager.java index 15ba503..972b364 100644 --- a/src/main/java/org/eclipse/persistence/internal/helper/ReadLockManager.java +++ b/src/main/java/org/eclipse/persistence/internal/helper/ReadLockManager.java @@ -14,10 +14,33 @@ // Oracle - initial API and implementation package org.eclipse.persistence.internal.helper; -import org.eclipse.persistence.internal.helper.type.ReadLockAcquisitionMetadata; +import static java.util.Collections.unmodifiableList; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Vector; -import java.util.*; +import org.eclipse.persistence.internal.helper.type.ReadLockAcquisitionMetadata; +/** + * This essential class tracks metadata about threads acquiring and releasing read locks. Such information becomes + * crucial during system deadlocks, allowing us to analyze the resources desired by all threads participating in the + * EclipseLink concurrency layer. + * + * When deadlocks occur, we need insights into: - Threads stuck while attempting to acquire write locks. - Threads + * engaged in object building and waiting within the release deferred lock process. - Threads that have acquired cache + * keys for reading but have not yet released them. + * + * This metadata contributes to the big picture of understanding all activities within the concurrency layer. + * + * Refer to {@link org.eclipse.persistence.internal.helper.ConcurrencyManager#READ_LOCK_MANAGERS}, a static hash map + * that provides details for each individual thread working in the concurrency layer, about the read locks it is + * currently acquiring for reading. + */ public class ReadLockManager { public static final int FIRST_INDEX_OF_COLLECTION = 0; @@ -28,35 +51,56 @@ public class ReadLockManager { * the readers count on a cache key whenever * */ - private final Vector readLocks = new Vector<>(1); + protected final Vector readLocks = new Vector<>(1); /** * We have seen that the read locks vector we have is very insufficient. We keep it for now due to the tracing code * that is making use of it. But we also want to have a new tracing map with a lot more metadata */ - private final Map> mapThreadToReadLockAcquisitionMetadata = new HashMap<>(); + protected final Map> mapThreadToReadLockAcquisitionMetadata = new HashMap<>(); /** * This is a field that will never be cleared. It will be getting ADDs if any problem is detected when we try remove * a read lock from our tracing. */ - private final List removeReadLockProblemsDetected = new ArrayList<>(); + protected final List removeReadLockProblemsDetected = new ArrayList<>(); /** * add a concurrency manager as deferred locks to the DLM */ public synchronized void addReadLock(ConcurrencyManager concurrencyManager) { + // (a) paranoia code - make sure we are not being provided a null concurrency manager ever + if(concurrencyManager == null) { + return ; + } + + // (b) Basic variable initializaton final Thread currentThread = Thread.currentThread(); final long currentThreadId = currentThread.getId(); ReadLockAcquisitionMetadata readLockAcquisitionMetadata = ConcurrencyUtil.SINGLETON.createReadLockAcquisitionMetadata(concurrencyManager); + // (c) update the read locks owned by the current thread this.readLocks.add(FIRST_INDEX_OF_COLLECTION, concurrencyManager); + + // (d) make sure our call to get the list of ReadLockAcquisitionMetadata mapThreadToReadLockAcquisitionMetadata for our current thread + // is never null. if(!mapThreadToReadLockAcquisitionMetadata.containsKey(currentThreadId)) { List newList = Collections.synchronizedList(new ArrayList()); mapThreadToReadLockAcquisitionMetadata.put(currentThreadId, newList ); } List acquiredReadLocksInCurrentTransactionList = mapThreadToReadLockAcquisitionMetadata.get(currentThreadId); - acquiredReadLocksInCurrentTransactionList.add(FIRST_INDEX_OF_COLLECTION, readLockAcquisitionMetadata); + + // NOTE: + // This if different than null check here should really not be needed + // This if check is a piece of paranoioa code. + // However we are adding it here because of a Null pointer exception + // org.eclipse.persistence.internal.helper.ReadLockManager.removeReadLock(ReadLockManager.java:84) + // whereby the conclusion of that NPException is that somehow our + // list of ReadLockAcquisitionMetadata was holding NULL entries within it + // this if will certainly not hurt us + if(readLockAcquisitionMetadata!=null) { + acquiredReadLocksInCurrentTransactionList.add(FIRST_INDEX_OF_COLLECTION, readLockAcquisitionMetadata); + } } /** @@ -68,19 +112,48 @@ public class ReadLockManager { * the concurrency cache key that is about to be decrement in number of readers. */ public synchronized void removeReadLock(ConcurrencyManager concurrencyManager) { + // (a) paranoia code - make sure we are not being provided a null concurrency manager ever + if(concurrencyManager == null) { + return ; + } + // (b) cleanup the concurrencyManager out of the vector of readlocks + this.readLocks.remove(concurrencyManager); + + // (c) basic variable initialization to work on the cleanup of the ReadLockAcquisitionMetadata final Thread currentThread = Thread.currentThread(); final long currentThreadId = currentThread.getId(); boolean readLockManagerHasTracingAboutAddedReadLocksForCurrentThread = mapThreadToReadLockAcquisitionMetadata.containsKey(currentThreadId); + // (d) make sure our call to get the list of ReadLockAcquisitionMetadata mapThreadToReadLockAcquisitionMetadata for our current thread + // is never null. if (!readLockManagerHasTracingAboutAddedReadLocksForCurrentThread) { String errorMessage = ConcurrencyUtil.SINGLETON.readLockManagerProblem02ReadLockManageHasNoEntriesForThread(concurrencyManager, currentThreadId); removeReadLockProblemsDetected.add(errorMessage); return; } + // (d) Search for the ReadLockAcquisitionMetadata associated to the current input cache key List readLocksAcquiredDuringCurrentThread = mapThreadToReadLockAcquisitionMetadata.get(currentThreadId); ReadLockAcquisitionMetadata readLockAquisitionMetadataToRemove = null; + boolean anyNullReadLockAcquisitionMetadataEntryFound = false; for (ReadLockAcquisitionMetadata currentReadLockAcquisitionMetadata : readLocksAcquiredDuringCurrentThread) { + // (d.i) + if(currentReadLockAcquisitionMetadata == null) { + // note: it should be impossible to enter this code area here + // the addReadLock appears to be rock solid, we cannot see any way to add a null value into the list of ReadLockAcquisitionMetadata + // however we have had a report of a null pointer exception doing currentReadLockAcquisitionMetadata getCacheKeyWhoseNumberOfReadersThreadIsIncrementing + // at: + // org.eclipse.persistence.internal.helper.ReadLockManager.removeReadLock(ReadLockManager.java:84) ~[eclipselink-2.7.6-weblogic.jar + // which means a NPE in the: + // line of code ConcurrencyManager currentCacheKeyObjectToCheck = currentReadLockAcquisitionMetadata.getCacheKeyWhoseNumberOfReadersThreadIsIncrementing(); + removeReadLockProblemsDetected.add("removeReadLock: we have detected the exisence of a NULL currentReadLockAcquisitionMetadata in the list readLocksAcquiredDuringCurrentThread. " + + " this should be impossible. The add logic code tries to make sure there is no scenario that would allow this to ever happen. "); + anyNullReadLockAcquisitionMetadataEntryFound = true; + // skip the next ReadLockAcquisitionMetadata entry in the list + continue; + } + + // (d.ii) check if the currentReadLockAcquisitionMetadata is the one we are trying to remove out the list ConcurrencyManager currentCacheKeyObjectToCheck = currentReadLockAcquisitionMetadata.getCacheKeyWhoseNumberOfReadersThreadIsIncrementing(); boolean dtoToRemoveFound = concurrencyManager.getConcurrencyManagerId() == currentCacheKeyObjectToCheck.getConcurrencyManagerId(); if (dtoToRemoveFound) { @@ -89,14 +162,24 @@ public class ReadLockManager { } } + // (e) Paranoia code - if our our list of ReadLockAcquisitionMetadata seems to be poisoned with null entries, get rid of those + if(anyNullReadLockAcquisitionMetadataEntryFound) { + readLocksAcquiredDuringCurrentThread.removeAll(Collections.singleton(null)); + readLocks.removeAll(Collections.singleton(null)); + } + + // (f) If we find no ReadLockAcquisitionMetadata associated to the input ConcurrencyManager there is a bug somewhere + // for each read lock in our vector of read locks we should also have added acquisition metadata if (readLockAquisitionMetadataToRemove == null) { String errorMessage = ConcurrencyUtil.SINGLETON.readLockManagerProblem03ReadLockManageHasNoEntriesForThread(concurrencyManager, currentThreadId); removeReadLockProblemsDetected.add(errorMessage); return; } - this.readLocks.remove(concurrencyManager); + + // (g) Happy path logic - get rid of the readLockAquisitionMetadataToRemove readLocksAcquiredDuringCurrentThread.remove(readLockAquisitionMetadataToRemove); + // (f) if our list goes empty and have no further readLockAquisitionMetadata then we can get rid of the list if (readLocksAcquiredDuringCurrentThread.isEmpty()) { mapThreadToReadLockAcquisitionMetadata.remove(currentThreadId); } @@ -106,7 +189,10 @@ public class ReadLockManager { * Return a set of the deferred locks */ public synchronized List getReadLocks() { - return Collections.unmodifiableList(readLocks); + // Simply returning an unmodifiable list is insufficient. + // We must provide a completely independent list to avoid issues when producing massive dumps. + // These dumps are iterated over in parallel with backend threads that are acquiring and releasing read locks. + return Collections.unmodifiableList(new ArrayList<>(readLocks)); } /** @@ -120,14 +206,37 @@ public class ReadLockManager { removeReadLockProblemsDetected.add(problemDetected); } - /** Getter for {@link #mapThreadToReadLockAcquisitionMetadata} */ - public Map> getMapThreadToReadLockAcquisitionMetadata() { - return mapThreadToReadLockAcquisitionMetadata; + /** Getter for {@link #mapThreadToReadLockAcquisitionMetadata} returns a deep clone */ + public synchronized Map> getMapThreadToReadLockAcquisitionMetadata() { + // We cannot simply return an unmodifiable map here. There are two reasons for this: + // 1. The code consuming this unmodifiable map might be surprised by changes to the map itself caused by this + // read lock manager. + // If threads continue to acquire and release read locks, it could impact this object. + // 2. Additionally, the list values contained in the map could also be affected by threads that are reading and + // releasing read locks. + // Our approach should be to provide the ConcurrencyUtil with a safe deep clone of the data structure for its + // massive dumps. + + // (a) Let's start by creating an independent result map that the caller can safely iterate over. + Map> resultMap = new HashMap<>(); + + // (b) depp clone the data strcuture + for (Entry> currentEntry : mapThreadToReadLockAcquisitionMetadata + .entrySet()) { + ArrayList deepCopyOfCurrentListOfMetadata = new ArrayList<>(currentEntry.getValue()); + resultMap.put(currentEntry.getKey(), unmodifiableList(deepCopyOfCurrentListOfMetadata) ); + } + + // (c) even if our result map is deep clone of our internal sate + // we still want to return it as unmodifable so that callers do not have the illusion + // they are able to hack the state of the read lock manager from the outside. + // If any code tris to manipulate the returned clone they should get a blow up to be dispelled of any illiusions + return Collections.unmodifiableMap(resultMap); } /** Getter for {@link #removeReadLockProblemsDetected} */ - public List getRemoveReadLockProblemsDetected() { - return removeReadLockProblemsDetected; + public synchronized List getRemoveReadLockProblemsDetected() { + return Collections.unmodifiableList(new ArrayList<>(removeReadLockProblemsDetected)) ; } /** diff --git a/src/test/java/org/eclipse/persistence/internal/helper/type/ReadLockAcquisitionMetadataTest.java b/src/test/java/org/eclipse/persistence/internal/helper/type/ReadLockAcquisitionMetadataTest.java new file mode 100644 index 0000000..3ee0442 --- /dev/null +++ b/src/test/java/org/eclipse/persistence/internal/helper/type/ReadLockAcquisitionMetadataTest.java @@ -0,0 +1,195 @@ +/* + * ----------------------------------------------------------------------------- + * manually + * Removed text manually + * Removed text manually + * ----------------------------------------------------------------------------- + */ +package org.eclipse.persistence.internal.helper.type; + +import java.util.List; + +import org.eclipse.persistence.internal.helper.ConcurrencyManager; +import org.eclipse.persistence.internal.helper.ReadLockManager; +import org.eclipse.persistence.internal.identitymaps.CacheKey; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +/** + * Unit test for the class {@link org.eclipse.persistence.internal.helper.ReadLockManager} + */ +public class ReadLockAcquisitionMetadataTest { + + // Inner class + /** + * Unit test extension class to allows to create some hacky methods to access + * and manipulate the state of the read lock manager. + * Such as adding NULL records into the state. + */ + public static class ReadLockManagerExtension extends ReadLockManager{ + + /** + * This is a dummy method hack our state maps with null entries. + * Our expectation is that this should never happen in real life + * but we have seen null pointer exceptions that seem to indicate that this is factuallz posisbly to happen + * although we have understoor the underlying reason for the null entries. + * + * With this method we simulate corrupting our read data structures with null entries. + */ + public void hackReadLockManagerToBeCorruptedWithNullRecordsItsState() { + ConcurrencyManager cacheKeyNull = null; + readLocks.add(cacheKeyNull); + final Thread currentThread = Thread.currentThread(); + final long currentThreadId = currentThread.getId(); + ReadLockAcquisitionMetadata readLockAcquisitionMetadataNull = null; + List readLocksAcquiredDuringCurrentThreadList = mapThreadToReadLockAcquisitionMetadata.get(currentThreadId); + readLocksAcquiredDuringCurrentThreadList.add(readLockAcquisitionMetadataNull); + + } + + + } + // helpver variables + + final ConcurrencyManager cacheKeyA = new CacheKey("cacheKeyA"); + final ConcurrencyManager cacheKeyB = new CacheKey("cacheKeyB"); + + + @Before + public void before() { + + } + + + @Test + public void normalHappyPathLogicAddingAndRemovingMetadataIntoTheReadLockManager() { + // SETUP: + // basic variable initialization + ReadLockManagerExtension testee = new ReadLockManagerExtension(); + + + // EXECUTE 1 - Add cache key A + testee.addReadLock(cacheKeyA); + + // VERIFY 1 + + Assert.assertEquals(1, testee.getReadLocks().size()); + Assert.assertTrue(testee.getReadLocks().contains(cacheKeyA)); + Assert.assertFalse(testee.getReadLocks().contains(cacheKeyB)); + + Assert.assertEquals(1, testee.getMapThreadToReadLockAcquisitionMetadata().size()); + List cacheKeyMetadataForCurrentThread =testee.getMapThreadToReadLockAcquisitionMetadata().get(Thread.currentThread().getId()); + Assert.assertEquals(1, cacheKeyMetadataForCurrentThread.size()); + + Assert.assertTrue(cacheKeyA == cacheKeyMetadataForCurrentThread.get(0).getCacheKeyWhoseNumberOfReadersThreadIsIncrementing()); + Assert.assertFalse(cacheKeyB == cacheKeyMetadataForCurrentThread.get(0).getCacheKeyWhoseNumberOfReadersThreadIsIncrementing()); + + + // EXECUTE 2 - Add cache key B + testee.addReadLock(cacheKeyB); + + // VERIFY 2 + + Assert.assertEquals(2, testee.getReadLocks().size()); + Assert.assertTrue(testee.getReadLocks().contains(cacheKeyA)); + Assert.assertTrue(testee.getReadLocks().contains(cacheKeyB)); + + Assert.assertEquals(1, testee.getMapThreadToReadLockAcquisitionMetadata().size()); + cacheKeyMetadataForCurrentThread =testee.getMapThreadToReadLockAcquisitionMetadata().get(Thread.currentThread().getId()); + Assert.assertEquals(2, cacheKeyMetadataForCurrentThread.size()); + + // note: when we are adding, we are adding the entries to the HEAD of the list + Assert.assertTrue(cacheKeyB == cacheKeyMetadataForCurrentThread.get(0).getCacheKeyWhoseNumberOfReadersThreadIsIncrementing()); + Assert.assertTrue(cacheKeyA == cacheKeyMetadataForCurrentThread.get(1).getCacheKeyWhoseNumberOfReadersThreadIsIncrementing()); + + + // EXECUTE 3 - Remove cache keys + testee.removeReadLock(cacheKeyA); + + // VERIFY 3 + Assert.assertEquals(1, testee.getReadLocks().size()); + Assert.assertFalse(testee.getReadLocks().contains(cacheKeyA)); + Assert.assertTrue(testee.getReadLocks().contains(cacheKeyB)); + + Assert.assertEquals(1, testee.getMapThreadToReadLockAcquisitionMetadata().size()); + cacheKeyMetadataForCurrentThread =testee.getMapThreadToReadLockAcquisitionMetadata().get(Thread.currentThread().getId()); + Assert.assertEquals(1, cacheKeyMetadataForCurrentThread.size()); + + Assert.assertTrue(cacheKeyB == cacheKeyMetadataForCurrentThread.get(0).getCacheKeyWhoseNumberOfReadersThreadIsIncrementing()); + Assert.assertFalse(cacheKeyA == cacheKeyMetadataForCurrentThread.get(0).getCacheKeyWhoseNumberOfReadersThreadIsIncrementing()); + + // EXECUTE 4 - Remove cache keys + testee.removeReadLock(cacheKeyB); + + // VERIFY 4 + Assert.assertEquals(0, testee.getReadLocks().size()); + Assert.assertFalse(testee.getReadLocks().contains(cacheKeyA)); + Assert.assertFalse(testee.getReadLocks().contains(cacheKeyB)); + Assert.assertEquals(0, testee.getMapThreadToReadLockAcquisitionMetadata().size()); + + + } + + @Test + public void testAddNullReadCacheKeyDoesNothing() { + // SETUP: + // basic variable initialization + ReadLockManagerExtension testee = new ReadLockManagerExtension(); + ConcurrencyManager cacheKeyNull = null; + + // EXECUTE + // try to add a null cache key to the map + testee.addReadLock(cacheKeyNull); + + // VERIFY + Assert.assertEquals(0, testee.getReadLocks().size()); + Assert.assertEquals(0, testee.getMapThreadToReadLockAcquisitionMetadata().size()); + } + + /** + * The purpose of this unit test is to make sure that if for some unknown reason + * we ever end up adding NULL as metadata to either the READ Lock manager + * or to the VECTOR of read locks + * that we can self heald and remove them from the map automatically. + * + */ + @Test + public void testRemoveWhen_mapThreadToReadLockAcquisitionMetadata_containsNull() { + // SETUP: + // let us start by adding some entrires to the read lock manager + ReadLockManagerExtension testee = new ReadLockManagerExtension(); + testee.addReadLock(cacheKeyA); + testee.addReadLock(cacheKeyB); + Assert.assertEquals(2, testee.getReadLocks().size()); + Assert.assertEquals(1, testee.getMapThreadToReadLockAcquisitionMetadata().size()); + List cacheKeyMetadataForCurrentThread =testee.getMapThreadToReadLockAcquisitionMetadata().get(Thread.currentThread().getId()); + Assert.assertEquals(2, cacheKeyMetadataForCurrentThread.size()); + + // SETUP: + // now we are going to hack our state to put in here null entires both in the read locks map and in the list of metadata + // Validate that that the maps are now properly hacked + testee.hackReadLockManagerToBeCorruptedWithNullRecordsItsState(); + Assert.assertEquals(3, testee.getReadLocks().size()); + Assert.assertTrue( testee.getReadLocks().contains(null)); + cacheKeyMetadataForCurrentThread =testee.getMapThreadToReadLockAcquisitionMetadata().get(Thread.currentThread().getId()); + Assert.assertEquals(3, cacheKeyMetadataForCurrentThread.size()); + Assert.assertTrue( cacheKeyMetadataForCurrentThread.contains(null)); + + // EXECUTE + // Now we will try to REMOVE from the read lock manager a cache key that does not actually exist in the map + // this MUST have the side effect of causing the code to loop over ALL OF THE read lock manager metadata and spot that we + // have NULL records in the metadata array + // in so doing the code should self-heal and get rid of all that garbage + ConcurrencyManager cacheKeyNotExistingInTheReadLockManagerToCallFullLoopOverData = new CacheKey("cacheKeyNotExistingInTheReadLockManagerToCallFullLoopOverData"); + testee.removeReadLock(cacheKeyNotExistingInTheReadLockManagerToCallFullLoopOverData); + + // VERIFY that our code self healeded + Assert.assertEquals(2, testee.getReadLocks().size()); + Assert.assertFalse( testee.getReadLocks().contains(null)); + cacheKeyMetadataForCurrentThread =testee.getMapThreadToReadLockAcquisitionMetadata().get(Thread.currentThread().getId()); + Assert.assertEquals(2, cacheKeyMetadataForCurrentThread.size()); + Assert.assertFalse( cacheKeyMetadataForCurrentThread.contains(null)); + + } +} -- 2.38.1.windows.1