[vmkit-commits] [PATCH/DISCUSS] registerNatives Lock, avoid deadlock

[Will Dietz]

Hi,

I'm noticing deadlocks occurring in the OpenJDK port involving the
registerNatives stuff.

Looking at the lock acquisition/release things look clean (the only
way to exit without releasing a lock is through an assertion
failure)--but I don't know what kinds of larger lock-related
dependencies/assumptions/orderings there might be.  Apparently the
existing code does do something wrong, however.

Inlined below is a patch that simply gives registerNatives its own
lock, which seems to resolve these issues in all cases (the deadlock
occurs reliably in the codes that trigger this).  However I don't like
"I don't know what was wrong before" bits, and thought I'd bounce this
off you before deep-diving the locking to see what's wrong.

FWIW the other locks held during deadlock are the protectIR() locks.

Thanks for your time! :)

~Will

>From d28307a26fdecdddb96273a813451e56216857ab Mon Sep 17 00:00:00 2001
From: Will Dietz <w at wdtz.org>
Date: Wed, 2 Nov 2011 17:19:31 -0500
Subject: [PATCH] Add new 'nativesLock' to protect the registerNatives map.

This fixes various deadlock scenarios when using the other lock.
Not sure what caused them, but this works for now and is conceptually
a different lock anyway.

Avoiding debugging the other version for now :).
---
 lib/J3/VMCore/JnjvmClassLoader.cpp |    8 ++++----
 lib/J3/VMCore/JnjvmClassLoader.h   |    4 ++++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/lib/J3/VMCore/JnjvmClassLoader.cpp
b/lib/J3/VMCore/JnjvmClassLoader.cpp
index 97b598e..59659c5 100644
--- a/lib/J3/VMCore/JnjvmClassLoader.cpp
+++ b/lib/J3/VMCore/JnjvmClassLoader.cpp
@@ -1046,17 +1046,17 @@ JavaString**
StringList::addString(JnjvmClassLoader* JCL, JavaString* obj) {
 }

 void JnjvmClassLoader::registerNative(JavaMethod * meth, word_t fnPtr) {
-  lock.lock();
+  nativesLock.lock();
   // Don't support multiple levels of registerNatives
   assert(registeredNatives.find(meth) == registeredNatives.end());

   registeredNatives[meth] = fnPtr;
-  lock.unlock();
+  nativesLock.unlock();
 }

 word_t JnjvmClassLoader::getRegisteredNative(const JavaMethod * meth) {
-  lock.lock();
+  nativesLock.lock();
   word_t res = registeredNatives[meth];
-  lock.unlock();
+  nativesLock.unlock();
   return res;
 }
diff --git a/lib/J3/VMCore/JnjvmClassLoader.h b/lib/J3/VMCore/JnjvmClassLoader.h
index f5891b0..3c1824b 100644
--- a/lib/J3/VMCore/JnjvmClassLoader.h
+++ b/lib/J3/VMCore/JnjvmClassLoader.h
@@ -113,6 +113,10 @@ protected:
   ///
   std::map<const JavaMethod*,word_t> registeredNatives;

+  /// nativesLock - Locks the registeredNatives map above
+  ///
+  mvm::LockRecursive nativesLock;
+
 public:

   /// allocator - Reference to the memory allocator, which will allocate UTF8s,
-- 
1.7.5.1