Author: jbellis Date: Wed Jul 27 21:00:28 2011 New Revision: 1151625 URL: http://svn.apache.org/viewvc?rev=1151625&view=rev Log: fix potential use of free'd native memory interface/SerializingCache patch by jbellis; reviewed by slebresne for CASSANDRA-2951
Modified: cassandra/trunk/CHANGES.txt cassandra/trunk/src/java/org/apache/cassandra/cache/FreeableMemory.java cassandra/trunk/src/java/org/apache/cassandra/cache/SerializingCache.java Modified: cassandra/trunk/CHANGES.txt URL: http://svn.apache.org/viewvc/cassandra/trunk/CHANGES.txt?rev=1151625&r1=1151624&r2=1151625&view=diff ============================================================================== --- cassandra/trunk/CHANGES.txt (original) +++ cassandra/trunk/CHANGES.txt Wed Jul 27 21:00:28 2011 @@ -22,6 +22,8 @@ * check column family validity in nodetool repair (CASSANDRA-2933) * use lazy initialization instead of class initialization in NodeId (CASSANDRA-2953) + * fix potential use of free'd native memory in SerializingCache + (CASSANDRA-1951) 0.8.2 Modified: cassandra/trunk/src/java/org/apache/cassandra/cache/FreeableMemory.java URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/cache/FreeableMemory.java?rev=1151625&r1=1151624&r2=1151625&view=diff ============================================================================== --- cassandra/trunk/src/java/org/apache/cassandra/cache/FreeableMemory.java (original) +++ cassandra/trunk/src/java/org/apache/cassandra/cache/FreeableMemory.java Wed Jul 27 21:00:28 2011 @@ -22,19 +22,40 @@ package org.apache.cassandra.cache; import java.io.IOException; +import java.util.concurrent.atomic.AtomicInteger; import com.sun.jna.Memory; public class FreeableMemory extends Memory { - protected volatile boolean valid = true; - + AtomicInteger references = new AtomicInteger(0); + public FreeableMemory(long size) { super(size); } - public void free() + /** @return true if we succeed in referencing before the reference count reaches zero */ + public boolean reference() + { + while (true) + { + int n = references.get(); + if (n <= 0) + return false; + if (references.compareAndSet(n, n + 1)) + return true; + } + } + + /** decrement reference count. if count reaches zero, the object is freed. */ + public void unreference() + { + if (references.decrementAndGet() == 0) + free(); + } + + private void free() { assert peer != 0; super.finalize(); // calls free and sets peer to zero @@ -46,8 +67,8 @@ public class FreeableMemory extends Memo @Override protected void finalize() { - if (peer != 0) - super.finalize(); + assert references.get() == 0; + assert peer == 0; } public byte getValidByte(long offset) Modified: cassandra/trunk/src/java/org/apache/cassandra/cache/SerializingCache.java URL: http://svn.apache.org/viewvc/cassandra/trunk/src/java/org/apache/cassandra/cache/SerializingCache.java?rev=1151625&r1=1151624&r2=1151625&view=diff ============================================================================== --- cassandra/trunk/src/java/org/apache/cassandra/cache/SerializingCache.java (original) +++ cassandra/trunk/src/java/org/apache/cassandra/cache/SerializingCache.java Wed Jul 27 21:00:28 2011 @@ -55,7 +55,7 @@ public class SerializingCache<K, V> impl { public void onEviction(K k, FreeableMemory mem) { - mem.free(); + mem.unreference(); } }; this.map = new ConcurrentLinkedHashMap.Builder<K, FreeableMemory>() @@ -137,7 +137,16 @@ public class SerializingCache<K, V> impl FreeableMemory mem = map.get(key); if (mem == null) return null; - return deserialize(mem); + if (!mem.reference()) + return null; + try + { + return deserialize(mem); + } + finally + { + mem.unreference(); + } } public void put(K key, V value) @@ -146,16 +155,17 @@ public class SerializingCache<K, V> impl if (mem == null) return; // out of memory. never mind. + mem.reference(); FreeableMemory old = map.put(key, mem); if (old != null) - old.free(); + old.unreference(); } public void remove(K key) { FreeableMemory mem = map.remove(key); if (mem != null) - mem.free(); + mem.unreference(); } public Set<K> keySet()