Oh yes, because getRegisteredNative is called while compiling, and the class loader lock may be used indirectly by application, we may have a deadlock situation.<div><br></div><div>Besides fixing the problem, I think your patch is cleaner because it makes the locking local. Apologies for suggesting you a bogus implementation. I'm surprised you encountered the bug that early though! Are you running vmkit w/openjdk on an application that creates mulitple threads?</div>
<div><br></div><div>Please apply!</div><div><br></div><div>Nicolas</div><div><br><div class="gmail_quote">On Thu, Nov 3, 2011 at 3:57 PM, Will Dietz <span dir="ltr"><<a href="mailto:wdietz2@illinois.edu">wdietz2@illinois.edu</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Hi,<br>
<br>
I'm noticing deadlocks occurring in the OpenJDK port involving the<br>
registerNatives stuff.<br>
<br>
Looking at the lock acquisition/release things look clean (the only<br>
way to exit without releasing a lock is through an assertion<br>
failure)--but I don't know what kinds of larger lock-related<br>
dependencies/assumptions/orderings there might be. Apparently the<br>
existing code does do something wrong, however.<br>
<br>
Inlined below is a patch that simply gives registerNatives its own<br>
lock, which seems to resolve these issues in all cases (the deadlock<br>
occurs reliably in the codes that trigger this). However I don't like<br>
"I don't know what was wrong before" bits, and thought I'd bounce this<br>
off you before deep-diving the locking to see what's wrong.<br>
<br>
FWIW the other locks held during deadlock are the protectIR() locks.<br>
<br>
Thanks for your time! :)<br>
<br>
~Will<br>
<br>
>From d28307a26fdecdddb96273a813451e56216857ab Mon Sep 17 00:00:00 2001<br>
From: Will Dietz <<a href="mailto:w@wdtz.org">w@wdtz.org</a>><br>
Date: Wed, 2 Nov 2011 17:19:31 -0500<br>
Subject: [PATCH] Add new 'nativesLock' to protect the registerNatives map.<br>
<br>
This fixes various deadlock scenarios when using the other lock.<br>
Not sure what caused them, but this works for now and is conceptually<br>
a different lock anyway.<br>
<br>
Avoiding debugging the other version for now :).<br>
---<br>
lib/J3/VMCore/JnjvmClassLoader.cpp | 8 ++++----<br>
lib/J3/VMCore/JnjvmClassLoader.h | 4 ++++<br>
2 files changed, 8 insertions(+), 4 deletions(-)<br>
<br>
diff --git a/lib/J3/VMCore/JnjvmClassLoader.cpp<br>
b/lib/J3/VMCore/JnjvmClassLoader.cpp<br>
index 97b598e..59659c5 100644<br>
--- a/lib/J3/VMCore/JnjvmClassLoader.cpp<br>
+++ b/lib/J3/VMCore/JnjvmClassLoader.cpp<br>
@@ -1046,17 +1046,17 @@ JavaString**<br>
StringList::addString(JnjvmClassLoader* JCL, JavaString* obj) {<br>
}<br>
<br>
void JnjvmClassLoader::registerNative(JavaMethod * meth, word_t fnPtr) {<br>
- lock.lock();<br>
+ nativesLock.lock();<br>
// Don't support multiple levels of registerNatives<br>
assert(registeredNatives.find(meth) == registeredNatives.end());<br>
<br>
registeredNatives[meth] = fnPtr;<br>
- lock.unlock();<br>
+ nativesLock.unlock();<br>
}<br>
<br>
word_t JnjvmClassLoader::getRegisteredNative(const JavaMethod * meth) {<br>
- lock.lock();<br>
+ nativesLock.lock();<br>
word_t res = registeredNatives[meth];<br>
- lock.unlock();<br>
+ nativesLock.unlock();<br>
return res;<br>
}<br>
diff --git a/lib/J3/VMCore/JnjvmClassLoader.h b/lib/J3/VMCore/JnjvmClassLoader.h<br>
index f5891b0..3c1824b 100644<br>
--- a/lib/J3/VMCore/JnjvmClassLoader.h<br>
+++ b/lib/J3/VMCore/JnjvmClassLoader.h<br>
@@ -113,6 +113,10 @@ protected:<br>
///<br>
std::map<const JavaMethod*,word_t> registeredNatives;<br>
<br>
+ /// nativesLock - Locks the registeredNatives map above<br>
+ ///<br>
+ mvm::LockRecursive nativesLock;<br>
+<br>
public:<br>
<br>
/// allocator - Reference to the memory allocator, which will allocate UTF8s,<br>
<span class="HOEnZb"><font color="#888888">--<br>
1.7.5.1<br>
_______________________________________________<br>
vmkit-commits mailing list<br>
<a href="mailto:vmkit-commits@cs.uiuc.edu">vmkit-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/vmkit-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/vmkit-commits</a><br>
</font></span></blockquote></div><br></div>