Skip to content

Commit 7a8c80f

Browse files
author
Johnu George
committed
MNEMONIC-215: Cleanup of memory leaks in Linkedlist impl
1 parent 1559100 commit 7a8c80f

File tree

3 files changed

+114
-37
lines changed

3 files changed

+114
-37
lines changed

‎mnemonic-collections/src/main/java/org/apache/mnemonic/collections/DurableHashMapImpl.java

Lines changed: 67 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import org.apache.mnemonic.EntityFactoryProxy;
2121
import org.apache.mnemonic.DurableType;
22+
import org.apache.mnemonic.Durable;
2223
import org.apache.mnemonic.MemChunkHolder;
2324
import org.apache.mnemonic.MemoryDurableEntity;
2425
import org.apache.mnemonic.OutOfHybridMemory;
@@ -81,7 +82,7 @@ public void setCapacityHint(long capacity) {
8182
* @return previous value with key else return null
8283
*/
8384
@Override
84-
public V put(K key, V value) {
85+
public V put(K key, V value) throws OutOfHybridMemory {
8586
int hash = hash(key.hashCode());
8687
long bucketIndex = getBucketIndex(hash);
8788
long bucketAddr = holder.get() + MAX_OBJECT_SIZE * bucketIndex;
@@ -106,13 +107,24 @@ public V put(K key, V value) {
106107
*
107108
* @return previous value with key else return null
108109
*/
109-
public V addEntry(K key, V value, long bucketAddr) {
110+
protected V addEntry(K key, V value, long bucketAddr) throws OutOfHybridMemory {
110111
V retValue = null;
111112
long handler = unsafe.getAddress(bucketAddr);
112113
if (0L == handler) {
113-
DurableSinglyLinkedList<MapEntry<K, V>> head = DurableSinglyLinkedListFactory.create(allocator,
114-
listefproxies, listgftypes, false);
115-
MapEntry<K, V> entry = MapEntryFactory.create(allocator, factoryProxy, genericField, false);
114+
DurableSinglyLinkedList<MapEntry<K, V>> head = null;
115+
MapEntry<K, V> entry = null;
116+
try {
117+
head = DurableSinglyLinkedListFactory.create(allocator, listefproxies, listgftypes, false);
118+
entry = MapEntryFactory.create(allocator, factoryProxy, genericField, false);
119+
} catch (OutOfHybridMemory fe) {
120+
if (null != head) {
121+
head.destroy();
122+
}
123+
if (null != entry) {
124+
entry.destroy();
125+
}
126+
throw fe;
127+
}
116128
entry.setKey(key, false);
117129
entry.setValue(value, false);
118130
head.setItem(entry, false);
@@ -128,17 +140,32 @@ public V addEntry(K key, V value, long bucketAddr) {
128140
K entryKey = mapEntry.getKey();
129141
if (entryKey == key || entryKey.equals(key)) {
130142
retValue = mapEntry.getValue();
131-
mapEntry.setValue(value, false);
143+
if (retValue instanceof Durable) {
144+
mapEntry.setValue(value, false);
145+
} else {
146+
mapEntry.setValue(value, true);
147+
}
132148
found = true;
133149
break;
134150
}
135151
prev = head;
136152
head = head.getNext();
137153
}
138154
if (true != found) {
139-
DurableSinglyLinkedList<MapEntry<K, V>> newNode = DurableSinglyLinkedListFactory.create(allocator,
140-
listefproxies, listgftypes, false);
141-
MapEntry<K, V> entry = MapEntryFactory.create(allocator, factoryProxy, genericField, false);
155+
DurableSinglyLinkedList<MapEntry<K, V>> newNode = null;
156+
MapEntry<K, V> entry = null;
157+
try {
158+
newNode = DurableSinglyLinkedListFactory.create(allocator, listefproxies, listgftypes, false);
159+
entry = MapEntryFactory.create(allocator, factoryProxy, genericField, false);
160+
} catch (OutOfHybridMemory fe) {
161+
if (null != newNode) {
162+
newNode.destroy();
163+
}
164+
if (null != entry) {
165+
entry.destroy();
166+
}
167+
throw fe;
168+
}
142169
entry.setKey(key, false);
143170
entry.setValue(value, false);
144171
newNode.setItem(entry, false);
@@ -176,7 +203,7 @@ public V get(K key) {
176203
*
177204
* @return previous value with key else return null
178205
*/
179-
public V getEntry(K key, long bucketAddr) {
206+
protected V getEntry(K key, long bucketAddr) {
180207
V retValue = null;
181208
long handler = unsafe.getAddress(bucketAddr);
182209
if (0L != handler) {
@@ -222,7 +249,7 @@ public V remove(K key) {
222249
*
223250
* @return previous value with key else return null
224251
*/
225-
public V removeEntry(K key, long bucketAddr) {
252+
protected V removeEntry(K key, long bucketAddr) {
226253
V retValue = null;
227254
long handler = unsafe.getAddress(bucketAddr);
228255
if (0L != handler) {
@@ -248,13 +275,11 @@ public V removeEntry(K key, long bucketAddr) {
248275
head.destroy();
249276
} else {
250277
unsafe.putAddress(bucketAddr, head.getNext().getHandler());
251-
head.setNext(null, false);
252-
head.destroy(); // #TODO: better way to delete one node
278+
head.destroy();
253279
}
254280
} else {
255281
prev.setNext(head.getNext(), false);
256-
head.setNext(null, false);
257-
head.destroy(); // #TODO: better way to delete one node
282+
head.destroy();
258283
}
259284
mapSize--;
260285
}
@@ -272,9 +297,14 @@ public void resize(long newCapacity) {
272297
MemChunkHolder<A> prevHolder = holder;
273298
long bucketAddr = prevHolder.get();
274299
long maxbucketAddr = bucketAddr + MAX_OBJECT_SIZE * totalCapacity;
300+
holder = allocator.createChunk(MAX_OBJECT_SIZE * newCapacity, autoReclaim);
301+
if (null == holder) {
302+
autoResize = false;
303+
holder = prevHolder;
304+
return;
305+
}
275306
totalCapacity = newCapacity;
276307
threshold = (long) (totalCapacity * DEFAULT_MAP_LOAD_FACTOR);
277-
holder = allocator.createChunk(MAX_OBJECT_SIZE * totalCapacity, autoReclaim);
278308
unsafe.putLong(chunkAddr.get(), allocator.getChunkHandler(holder));
279309
while (bucketAddr < maxbucketAddr) {
280310
long handler = unsafe.getAddress(bucketAddr);
@@ -299,7 +329,7 @@ public void resize(long newCapacity) {
299329
* @param elem
300330
* the item in the old map
301331
*/
302-
public void transfer(DurableSinglyLinkedList<MapEntry<K, V>> elem) {
332+
protected void transfer(DurableSinglyLinkedList<MapEntry<K, V>> elem) {
303333
int hash = hash(elem.getItem().getKey().hashCode());
304334
long bucketIndex = getBucketIndex(hash);
305335
long bucketAddr = holder.get() + MAX_OBJECT_SIZE * bucketIndex;
@@ -318,7 +348,7 @@ public void transfer(DurableSinglyLinkedList<MapEntry<K, V>> elem) {
318348
* Recomputes the size of the map during restore without persistence
319349
*
320350
*/
321-
public long recomputeMapSize() {
351+
protected long recomputeMapSize() {
322352
long size = 0;
323353
long bucketAddr = holder.get();
324354
long maxbucketAddr = bucketAddr + MAX_OBJECT_SIZE * totalCapacity;
@@ -351,13 +381,19 @@ public long[][] getNativeFieldInfo() {
351381
public void destroy() throws RetrieveDurableEntityError {
352382
long bucketAddr = holder.get();
353383
long maxbucketAddr = bucketAddr + MAX_OBJECT_SIZE * totalCapacity;
384+
DurableSinglyLinkedList<MapEntry<K, V>> head, prev;
354385
while (bucketAddr < maxbucketAddr) {
355386
long handler = unsafe.getAddress(bucketAddr);
356387
if (0L != handler) {
357-
DurableSinglyLinkedList<MapEntry<K, V>> head = DurableSinglyLinkedListFactory.restore(allocator,
388+
head = DurableSinglyLinkedListFactory.restore(allocator,
358389
listefproxies, listgftypes, handler, false);
359-
head.destroy();
390+
prev = head;
391+
while (null != head) {
392+
head = head.getNext();
393+
prev.destroy(); //TODO: Destroy head in a cascading way
394+
prev = head;
360395
}
396+
}
361397
bucketAddr += MAX_OBJECT_SIZE;
362398
}
363399
holder.destroy();
@@ -383,12 +419,12 @@ public long getHandler() {
383419

384420
@Override
385421
public void restoreDurableEntity(A allocator, EntityFactoryProxy[] factoryProxy,
386-
DurableType[] gField, long phandler, boolean autoreclaim) throws RestoreDurableEntityError {
387-
initializeDurableEntity(allocator, factoryProxy, gField, autoreclaim);
422+
DurableType[] gField, long phandler, boolean autoReclaim) throws RestoreDurableEntityError {
423+
initializeDurableEntity(allocator, factoryProxy, gField, autoReclaim);
388424
if (0L == phandler) {
389425
throw new RestoreDurableEntityError("Input handler is null on restoreDurableEntity.");
390426
}
391-
chunkAddr = allocator.retrieveChunk(phandler, autoreclaim);
427+
chunkAddr = allocator.retrieveChunk(phandler, autoReclaim);
392428
long chunkHandler = unsafe.getLong(chunkAddr.get());
393429
holder = allocator.retrieveChunk(chunkHandler, autoReclaim);
394430
if (null == holder || null == chunkAddr) {
@@ -436,10 +472,10 @@ public <A extends RestorableAllocator<A>> MapEntry<K, V> create(
436472

437473
@Override
438474
public void createDurableEntity(A allocator, EntityFactoryProxy[] factoryProxy,
439-
DurableType[] gField, boolean autoreclaim) throws OutOfHybridMemory {
440-
initializeDurableEntity(allocator, factoryProxy, gField, autoreclaim);
441-
this.holder = allocator.createChunk(MAX_OBJECT_SIZE * totalCapacity, autoreclaim);
442-
this.chunkAddr = allocator.createChunk(MAX_OBJECT_SIZE, autoreclaim);
475+
DurableType[] gField, boolean autoReclaim) throws OutOfHybridMemory {
476+
initializeDurableEntity(allocator, factoryProxy, gField, autoReclaim);
477+
this.holder = allocator.createChunk(MAX_OBJECT_SIZE * totalCapacity, autoReclaim);
478+
this.chunkAddr = allocator.createChunk(MAX_OBJECT_SIZE, autoReclaim);
443479
unsafe.putLong(chunkAddr.get(), allocator.getChunkHandler(holder));
444480
if (null == this.holder || null == this.chunkAddr) {
445481
throw new OutOfHybridMemory("Create Durable Entity Error!");
@@ -515,13 +551,13 @@ public void remove() {
515551
prevNode.destroy();
516552
} else {
517553
unsafe.putAddress(prevBucketAddr, prevNode.getNext().getHandler());
518-
prevNode.setNext(null, false);
519-
prevNode.destroy(); // #TODO: better way to delete one node
554+
prevNode.destroy();
555+
prevNode = null;
520556
}
521557
} else {
522558
prevPrevNode.setNext(prevNode.getNext(), false);
523-
prevNode.setNext(null, false);
524-
prevNode.destroy(); // #TODO: better way to delete one node
559+
prevNode.destroy();
560+
prevNode = prevPrevNode;
525561
}
526562
map.mapSize--;
527563
}

‎mnemonic-collections/src/test/java/org/apache/mnemonic/collections/DurableHashMapNGTest.java

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public void setUp() {
5353
public boolean reclaim(ByteBuffer mres, Long sz) {
5454
System.out.println(String.format("Reclaim Memory Buffer: %X Size: %s", System.identityHashCode(mres),
5555
null == sz ? "NULL" : sz.toString()));
56+
System.out.println(" String buffer " + mres.asCharBuffer().toString());
5657
return false;
5758
}
5859
});
@@ -136,6 +137,8 @@ public void testGetPutRemovePrimitives() {
136137
AssertJUnit.assertNull(val);
137138
val = restoredMap.get("test");
138139
AssertJUnit.assertEquals(4, val.intValue());
140+
141+
restoredMap.destroy();
139142
}
140143

141144
@Test(enabled = true)
@@ -189,6 +192,8 @@ public <A extends RestorableAllocator<A>> Person<Long> create(
189192

190193
str = map.get(third);
191194
AssertJUnit.assertEquals(str, "world");
195+
third.destroy();
196+
map.destroy();
192197
}
193198

194199
@Test(enabled = true)
@@ -211,13 +216,31 @@ public <A extends RestorableAllocator<A>> Person<Long> create(
211216
} };
212217

213218
Person<Long> person = (Person<Long>) efproxies[1].create(m_act, null, null, false);
219+
person.setName("Alice", false);
214220
person.setAge((short) 31);
215221
DurableHashMap<String, Person<Long>> map = DurableHashMapFactory.create(m_act,
216222
efproxies, gtypes, mInitialCapacity, false);
217223
map.put("hello", person);
218-
224+
Person<Long> anotherPerson = (Person<Long>) efproxies[1].create(m_act, null, null, false);
225+
anotherPerson.setAge((short) 30);
226+
anotherPerson.setName("Bob", false);
227+
map.put("world", anotherPerson);
228+
219229
Person<Long> per = map.get("hello");
220-
AssertJUnit.assertEquals(31, (int)per.getAge());
230+
AssertJUnit.assertEquals(31, (int)per.getAge());
231+
232+
per = map.get("world");
233+
AssertJUnit.assertEquals(30, (int)per.getAge());
234+
235+
Person<Long> third = (Person<Long>) efproxies[1].create(m_act, null, null, false);
236+
third.setAge((short) 29);
237+
third.setName("Frank", false);
238+
per = map.put("world", third);
239+
per.destroy();
240+
241+
per = map.get("world");
242+
AssertJUnit.assertEquals(29, (int)per.getAge());
243+
map.destroy();
221244
}
222245

223246
@Test(enabled = true)
@@ -251,7 +274,7 @@ public <A extends RestorableAllocator<A>> Person<Long> create(
251274
return PersonFactory.create(allocator, factoryproxys, gfields, autoreclaim);
252275
}
253276
} };
254-
277+
255278
Person<Long> person = (Person<Long>) efproxies[0].create(m_act, null, null, false);
256279
person.setAge((short) 31);
257280
person.setName("Bob", true);
@@ -263,11 +286,13 @@ public <A extends RestorableAllocator<A>> Person<Long> create(
263286
DurableHashMap<Person<Long>, Person<Long>> map = DurableHashMapFactory.create(m_act,
264287
efproxies, gtypes, mInitialCapacity, false);
265288
map.put(person, anotherPerson);
266-
289+
267290
Person<Long> per = map.get(person);
268291
AssertJUnit.assertEquals(30, (int)per.getAge());
269292
per = map.get(anotherPerson);
270293
AssertJUnit.assertNull(per);
294+
295+
map.destroy();
271296
}
272297

273298
@Test(enabled = true)
@@ -323,7 +348,7 @@ public <A extends RestorableAllocator<A>> Person<Long> create(
323348
person.setName("Bob", true);
324349

325350

326-
Person<Long> anotherPerson = (Person<Long>) efproxies[1].create(m_act, null, null, false);
351+
Person<Long> anotherPerson = PersonFactory.create(m_act, null, null, false);
327352
anotherPerson.setAge((short) 30);
328353
anotherPerson.setName("Alice", true);
329354

@@ -338,9 +363,12 @@ public <A extends RestorableAllocator<A>> Person<Long> create(
338363
bigMap.put("hello", map);
339364
per = bigMap.get("hello").get("world");
340365
AssertJUnit.assertEquals(31, (int)per.getAge());
366+
341367
bigMap.get("hello").put("testing", anotherPerson);
342368
per = bigMap.get("hello").get("testing");
343369
AssertJUnit.assertEquals("Alice", per.getName());
370+
371+
bigMap.destroy();
344372
}
345373

346374
@Test(enabled = true)
@@ -421,7 +449,7 @@ public void testAutoResizeMaps() {
421449
}
422450
AssertJUnit.assertEquals(count, 50);
423451

424-
452+
restoredMap.destroy();
425453
}
426454

427455
@Test(enabled = true)
@@ -470,5 +498,16 @@ public void testMapIterator() {
470498
AssertJUnit.assertEquals(map.get("hello").intValue(), 1);
471499
AssertJUnit.assertEquals(map.get("world").intValue(), 2);
472500
AssertJUnit.assertEquals(map.getSize(), 2);
501+
502+
iter = map.iterator();
503+
count = 0;
504+
while (iter.hasNext()) {
505+
entry = iter.next();
506+
count++;
507+
}
508+
AssertJUnit.assertEquals(count, 2);
509+
510+
map.destroy();
473511
}
512+
474513
}

‎mnemonic-core/src/main/java/org/apache/mnemonic/NonVolatileMemAllocator.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,7 @@ public DurableBuffer<NonVolatileMemAllocator> retrieveBuffer(long phandler, bool
300300
ByteBuffer bb = m_nvmasvc.retrieveByteBuffer(m_nid, getEffectiveAddress(phandler));
301301
if (null != bb) {
302302
ret = new DurableBuffer<NonVolatileMemAllocator>(this, bb);
303+
ret.setCollector(m_bufcollector);
303304
if (autoreclaim) {
304305
m_bufcollector.register(ret);
305306
}
@@ -326,6 +327,7 @@ public DurableChunk<NonVolatileMemAllocator> retrieveChunk(long phandler, boolea
326327
long sz = m_nvmasvc.retrieveSize(m_nid, eaddr);
327328
if (sz > 0L) {
328329
ret = new DurableChunk<NonVolatileMemAllocator>(this, eaddr, sz);
330+
ret.setCollector(m_chunkcollector);
329331
if (autoreclaim) {
330332
m_chunkcollector.register(ret);
331333
}

0 commit comments

Comments
 (0)