Avoid deadlock.

dvmIsHeapAddress is the pointer validity checker for use outside the GC. If
it doesn't take the heap lock, it risks looking at data structures that are
changing under its feet. If it does take the heap lock, it risks deadlock
between a thread doing an explicit GC and the signal catcher trying to dump
threads (causing the GC thread to be suspended while holding the heap lock).

Calling back into managed code while holding the heap lock and with other
threads resumed sounds like an inherently bad idea to me, but that's a battle
for another day.

With this change, we can handle SIGQUIT while we're in ReferenceQueue.add
as a result of an explicit GC in a system doing concurrent collections, so
the only known problem is fixed.

Bug: 5425802
Change-Id: I42d434d5ea3ffbcb77a4c544b81b08a4c7364a16

2 files changed, 4 insertions, 5 deletions

diff --git a/vm/alloc/Alloc.cpp b/vm/alloc/Alloc.cpp
index b37b9e1cc..d2c3336f8 100644
--- a/vm/alloc/Alloc.cpp
+++ b/vm/alloc/Alloc.cpp
@@ -350,10 +350,7 @@ size_t dvmCountAssignableInstancesOfClass(const ClassObject *clazz)
 
 bool dvmIsHeapAddress(void *address)
 {
-    dvmLockHeap();
-    bool result = dvmHeapSourceContainsAddress(address);
-    dvmUnlockHeap();
-    return result;
+    return address != NULL && (((uintptr_t) address & (8-1)) == 0);
 }
 
 bool dvmIsNonMovingObject(const Object* object)
diff --git a/vm/alloc/Alloc.h b/vm/alloc/Alloc.h
index b183f4019..efee1bde3 100644
--- a/vm/alloc/Alloc.h
+++ b/vm/alloc/Alloc.h
@@ -140,7 +140,9 @@ size_t dvmCountAssignableInstancesOfClass(const ClassObject *clazz);
 void dvmClearGrowthLimit(void);
 
 /*
- * Returns true if the address is within the bounds of the heap.
+ * Returns true if the address is aligned appropriately for a heap object.
+ * Does not require the caller to hold the heap lock, and does not take the
+ * heap lock internally.
  */
 bool dvmIsHeapAddress(void *address);