Fix concurrency bugs in BCC JIT cache.
ContextManager could potentially be accessed by multiple threads
so we now use a Mutex.
Fixed a bug in ContextManager where it could mark a slot as having
been assigned even though the subsequent call to mmap failed.
Added code to delete the cache file before recreating it. The existing
file may still be mmapped and executing somewhere else so we don't want
to modify its contents.
Bug: 3345334
Change-Id: I9c8382d9695741b92a4f66c4529ebb5d9a95f1d9
diff --git a/Android.mk b/Android.mk
index d194c53..4884928 100644
--- a/Android.mk
+++ b/Android.mk
@@ -159,6 +159,7 @@
LOCAL_STATIC_LIBRARIES := \
libcutils \
+ libutils \
libLLVMX86CodeGen \
libLLVMX86Info \
libLLVMARMCodeGen \
diff --git a/lib/bcc/ContextManager.cpp b/lib/bcc/ContextManager.cpp
index 917d7f1..ea8c8b1 100644
--- a/lib/bcc/ContextManager.cpp
+++ b/lib/bcc/ContextManager.cpp
@@ -21,6 +21,7 @@
#include <errno.h>
#include <sys/mman.h>
+#include <utils/threads.h>
#include <stddef.h>
#include <string.h>
@@ -33,25 +34,26 @@
namespace bcc {
+static android::Mutex gContextLock;
+
char *allocateContext() {
+ android::AutoMutex _l(gContextLock);
+
// Try to allocate context on the managed context slot.
for (size_t i = 0; i < BCC_CONTEXT_SLOT_COUNT; ++i) {
if (ContextSlotTaken[i]) {
continue;
}
- // Take the context slot. (No matter we can mmap or not)
- ContextSlotTaken[i] = true;
-
- void *addr = BCC_CONTEXT_FIXED_ADDR + BCC_CONTEXT_SIZE * i;
-
// Try to mmap
+ void *addr = BCC_CONTEXT_FIXED_ADDR + BCC_CONTEXT_SIZE * i;
void *result = mmap(addr, BCC_CONTEXT_SIZE,
PROT_READ | PROT_WRITE | PROT_EXEC,
MAP_PRIVATE | MAP_ANON, -1, 0);
if (result == addr) {
LOGI("Allocate bcc context. addr=%p\n", result);
+ ContextSlotTaken[i] = true;
return static_cast<char *>(result);
}
@@ -78,8 +80,22 @@
return (char *)result;
}
+static ssize_t getSlotIndexFromAddress(char* addr) {
+ if (addr >= BCC_CONTEXT_FIXED_ADDR) {
+ size_t offset = (size_t)(addr - BCC_CONTEXT_FIXED_ADDR);
+ if (offset % BCC_CONTEXT_SIZE == 0) {
+ size_t slot = offset / BCC_CONTEXT_SIZE;
+ if (slot < BCC_CONTEXT_SLOT_COUNT) {
+ return slot;
+ }
+ }
+ }
+ return -1;
+}
char *allocateContext(char *addr, int imageFd, off_t imageOffset) {
+ android::AutoMutex _l(gContextLock);
+
// This function should only allocate context when address is an context
// slot address. And the image offset is aligned to the pagesize.
@@ -96,15 +112,8 @@
return NULL;
}
- if (addr < BCC_CONTEXT_FIXED_ADDR) {
- LOGE("Suggested address is not a bcc context slot address\n");
- return NULL;
- }
-
- size_t offset = (size_t)(addr - BCC_CONTEXT_FIXED_ADDR);
- size_t slot = offset / BCC_CONTEXT_SIZE;
-
- if (offset % BCC_CONTEXT_SIZE != 0 || slot >= BCC_CONTEXT_SLOT_COUNT) {
+ ssize_t slot = getSlotIndexFromAddress(addr);
+ if (slot < 0) {
LOGE("Suggested address is not a bcc context slot address\n");
return NULL;
}
@@ -114,8 +123,6 @@
return NULL;
}
- ContextSlotTaken[slot] = true;
-
// LOGI("addr=%x, imageFd=%d, imageOffset=%x", addr, imageFd, imageOffset);
void *result = mmap(addr, BCC_CONTEXT_SIZE,
PROT_READ | PROT_WRITE | PROT_EXEC,
@@ -133,6 +140,7 @@
}
LOGI("Allocate bcc context. addr=%p\n", addr);
+ ContextSlotTaken[slot] = true;
return static_cast<char *>(result);
}
@@ -142,6 +150,8 @@
return;
}
+ android::AutoMutex _l(gContextLock);
+
LOGI("Deallocate bcc context. addr=%p\n", addr);
// Unmap
@@ -152,14 +162,10 @@
// If the address is one of the context slot, then mark such slot
// freely available as well.
- if (addr >= BCC_CONTEXT_FIXED_ADDR) {
- size_t offset = (size_t)(addr - BCC_CONTEXT_FIXED_ADDR);
- size_t slot = offset / BCC_CONTEXT_SIZE;
-
- if (offset % BCC_CONTEXT_SIZE == 0 && slot < BCC_CONTEXT_SLOT_COUNT) {
- // Give the context slot back.
- ContextSlotTaken[slot] = false;
- }
+ ssize_t slot = getSlotIndexFromAddress(addr);
+ if (slot >= 0) {
+ // Give the context slot back.
+ ContextSlotTaken[slot] = false;
}
}
diff --git a/lib/bcc/Script.cpp b/lib/bcc/Script.cpp
index ac74118..74b4793 100644
--- a/lib/bcc/Script.cpp
+++ b/lib/bcc/Script.cpp
@@ -254,6 +254,12 @@
if (mCachePath && !getBooleanProp("debug.bcc.nocache")) {
FileHandle file;
+ // Remove the file if it already exists before writing the new file.
+ // The old file may still be mapped elsewhere in memory and we do not want
+ // to modify its contents. (The same script may be running concurrently in
+ // the same process or a different process!)
+ ::unlink(mCachePath);
+
if (file.open(mCachePath, OpenMode::Write) >= 0) {
CacheWriter writer;