carbondata-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ravipes...@apache.org
Subject carbondata git commit: [CARBONDATA-1393] Avoid to throw NPE when execute 'freeMemory' of UnsafeMemoryManager/UnsafeSortMemoryManager
Date Wed, 30 Aug 2017 11:16:16 GMT
Repository: carbondata
Updated Branches:
  refs/heads/master 8ea7272d9 -> 289232607


[CARBONDATA-1393] Avoid to throw NPE when execute 'freeMemory' of UnsafeMemoryManager/UnsafeSortMemoryManager

UnsafeMemoryManager.freeMemoryAll(long taskId) may run before freeMemory(long taskId, MemoryBlock
memoryBlock), so taskIdToMemoryBlockMap.get(taskId) will return null and then throw NPE.

This closes #1266


Project: http://git-wip-us.apache.org/repos/asf/carbondata/repo
Commit: http://git-wip-us.apache.org/repos/asf/carbondata/commit/28923260
Tree: http://git-wip-us.apache.org/repos/asf/carbondata/tree/28923260
Diff: http://git-wip-us.apache.org/repos/asf/carbondata/diff/28923260

Branch: refs/heads/master
Commit: 289232607ae51f350c0dc13dcbb9da7b95491c11
Parents: 8ea7272
Author: Zhang Zhichao <441586683@qq.com>
Authored: Fri Aug 18 17:04:50 2017 +0800
Committer: Ravindra Pesala <ravi.pesala@gmail.com>
Committed: Wed Aug 30 16:45:52 2017 +0530

----------------------------------------------------------------------
 .../core/memory/HeapMemoryAllocator.java        |  3 +++
 .../carbondata/core/memory/MemoryBlock.java     | 14 +++++++++++
 .../core/memory/UnsafeMemoryAllocator.java      |  1 +
 .../core/memory/UnsafeMemoryManager.java        | 24 +++++++++++-------
 .../core/memory/UnsafeSortMemoryManager.java    | 26 ++++++++++++--------
 5 files changed, 49 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/carbondata/blob/28923260/core/src/main/java/org/apache/carbondata/core/memory/HeapMemoryAllocator.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/memory/HeapMemoryAllocator.java
b/core/src/main/java/org/apache/carbondata/core/memory/HeapMemoryAllocator.java
index 2b63726..5862933 100644
--- a/core/src/main/java/org/apache/carbondata/core/memory/HeapMemoryAllocator.java
+++ b/core/src/main/java/org/apache/carbondata/core/memory/HeapMemoryAllocator.java
@@ -53,6 +53,8 @@ public class HeapMemoryAllocator implements MemoryAllocator {
             final MemoryBlock memory = blockReference.get();
             if (memory != null) {
               assert (memory.size() == size);
+              // reuse this MemoryBlock
+              memory.setFreedStatus(false);
               return memory;
             }
           }
@@ -76,5 +78,6 @@ public class HeapMemoryAllocator implements MemoryAllocator {
         pool.add(new WeakReference<>(memory));
       }
     }
+    memory.setFreedStatus(true);
   }
 }

http://git-wip-us.apache.org/repos/asf/carbondata/blob/28923260/core/src/main/java/org/apache/carbondata/core/memory/MemoryBlock.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/memory/MemoryBlock.java b/core/src/main/java/org/apache/carbondata/core/memory/MemoryBlock.java
index 74136c1..d6cb184 100644
--- a/core/src/main/java/org/apache/carbondata/core/memory/MemoryBlock.java
+++ b/core/src/main/java/org/apache/carbondata/core/memory/MemoryBlock.java
@@ -29,9 +29,15 @@ public class MemoryBlock extends MemoryLocation {
 
   private final long length;
 
+  /**
+   * whether freed or not
+   */
+  private boolean isFreed = false;
+
   public MemoryBlock(@Nullable Object obj, long offset, long length) {
     super(obj, offset);
     this.length = length;
+    this.isFreed = false;
   }
 
   /**
@@ -41,6 +47,14 @@ public class MemoryBlock extends MemoryLocation {
     return length;
   }
 
+  public boolean isFreedStatus() {
+    return this.isFreed;
+  }
+
+  public void setFreedStatus(boolean freedStatus) {
+    this.isFreed = freedStatus;
+  }
+
   /**
    * Creates a memory block pointing to the memory used by the long array.
    */

http://git-wip-us.apache.org/repos/asf/carbondata/blob/28923260/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryAllocator.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryAllocator.java
b/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryAllocator.java
index fcb0b88..67412ac 100644
--- a/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryAllocator.java
+++ b/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryAllocator.java
@@ -36,5 +36,6 @@ public class UnsafeMemoryAllocator implements MemoryAllocator {
     assert (memory.obj == null) :
       "baseObject not null; are you trying to use the off-heap allocator to free on-heap
memory?";
     CarbonUnsafe.getUnsafe().freeMemory(memory.offset);
+    memory.setFreedStatus(true);
   }
 }

http://git-wip-us.apache.org/repos/asf/carbondata/blob/28923260/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryManager.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryManager.java
b/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryManager.java
index 1190d56..06f907d 100644
--- a/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryManager.java
+++ b/core/src/main/java/org/apache/carbondata/core/memory/UnsafeMemoryManager.java
@@ -105,13 +105,17 @@ public class UnsafeMemoryManager {
   }
 
   public synchronized void freeMemory(long taskId, MemoryBlock memoryBlock) {
-    taskIdToMemoryBlockMap.get(taskId).remove(memoryBlock);
-    allocator.free(memoryBlock);
-    memoryUsed -= memoryBlock.size();
-    memoryUsed = memoryUsed < 0 ? 0 : memoryUsed;
-    LOGGER.info(
-        "Freeing memory of size: " + memoryBlock.size() + "available memory:  " + (totalMemory
-            - memoryUsed));
+    if (taskIdToMemoryBlockMap.containsKey(taskId)) {
+      taskIdToMemoryBlockMap.get(taskId).remove(memoryBlock);
+    }
+    if (!memoryBlock.isFreedStatus()) {
+      allocator.free(memoryBlock);
+      memoryUsed -= memoryBlock.size();
+      memoryUsed = memoryUsed < 0 ? 0 : memoryUsed;
+      LOGGER.info(
+          "Freeing memory of size: " + memoryBlock.size() + "available memory:  " + (totalMemory
+              - memoryUsed));
+    }
   }
 
   public synchronized void freeMemoryAll(long taskId) {
@@ -123,8 +127,10 @@ public class UnsafeMemoryManager {
       MemoryBlock memoryBlock = null;
       while (iterator.hasNext()) {
         memoryBlock = iterator.next();
-        occuppiedMemory += memoryBlock.size();
-        allocator.free(memoryBlock);
+        if (!memoryBlock.isFreedStatus()) {
+          occuppiedMemory += memoryBlock.size();
+          allocator.free(memoryBlock);
+        }
       }
     }
     memoryUsed -= occuppiedMemory;

http://git-wip-us.apache.org/repos/asf/carbondata/blob/28923260/core/src/main/java/org/apache/carbondata/core/memory/UnsafeSortMemoryManager.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/carbondata/core/memory/UnsafeSortMemoryManager.java
b/core/src/main/java/org/apache/carbondata/core/memory/UnsafeSortMemoryManager.java
index b4a200a..c63b320 100644
--- a/core/src/main/java/org/apache/carbondata/core/memory/UnsafeSortMemoryManager.java
+++ b/core/src/main/java/org/apache/carbondata/core/memory/UnsafeSortMemoryManager.java
@@ -135,14 +135,18 @@ public class UnsafeSortMemoryManager {
   }
 
   public synchronized void freeMemory(long taskId, MemoryBlock memoryBlock) {
-    allocator.free(memoryBlock);
-    taskIdToMemoryBlockMap.get(taskId).remove(memoryBlock);
-    memoryUsed -= memoryBlock.size();
-    memoryUsed = memoryUsed < 0 ? 0 : memoryUsed;
-    if (LOGGER.isDebugEnabled()) {
-      LOGGER.debug(
-          "Freeing memory of size: " + memoryBlock.size() + ": Current available memory is:
" + (
-              totalMemory - memoryUsed));
+    if (taskIdToMemoryBlockMap.containsKey(taskId)) {
+      taskIdToMemoryBlockMap.get(taskId).remove(memoryBlock);
+    }
+    if (!memoryBlock.isFreedStatus()) {
+      allocator.free(memoryBlock);
+      memoryUsed -= memoryBlock.size();
+      memoryUsed = memoryUsed < 0 ? 0 : memoryUsed;
+      if (LOGGER.isDebugEnabled()) {
+        LOGGER.debug(
+            "Freeing memory of size: " + memoryBlock.size() + ": Current available memory
is: " + (
+                totalMemory - memoryUsed));
+      }
     }
   }
 
@@ -161,8 +165,10 @@ public class UnsafeSortMemoryManager {
       MemoryBlock memoryBlock = null;
       while (iterator.hasNext()) {
         memoryBlock = iterator.next();
-        occuppiedMemory += memoryBlock.size();
-        allocator.free(memoryBlock);
+        if (!memoryBlock.isFreedStatus()) {
+          occuppiedMemory += memoryBlock.size();
+          allocator.free(memoryBlock);
+        }
       }
     }
     memoryUsed -= occuppiedMemory;


Mime
View raw message