This is the mail archive of the
java-patches@gcc.gnu.org
mailing list for the Java project.
[RFA/JVMTI] Better env list locking
- From: Keith Seitz <keiths at redhat dot com>
- To: Java Patch List <java-patches at gcc dot gnu dot org>
- Date: Thu, 01 Feb 2007 16:37:56 -0800
- Subject: [RFA/JVMTI] Better env list locking
Hi,
A long time ago, I noted errors in the environment list locking in
jvmti.cc. Well, at long last, the time has come to fix it. With the
recent merge, we now have access to ReentrantReadWriteLock, which is
what is now used to lock the environment list.
Once again, this patch looks a lot bigger than it is. Much of it just
moves some chunks of code around.
Comments/questions/concerns?
Keith
ChangeLog
2007-02-01 Keith Seitz <keiths@redhat.com>
* jvmti.cc (_envListLock): Change type to
ReentrantReadWriteLock.
(_Jv_JVMTI_DisposeEnvironment): Switch to read/write
lock.
(check_enabled_event): Likewise.
(_Jv_GetJVMTIEnv): Likewise.
(_Jv_JVMTI_Init): Likewise.
(_Jv_JVMTI_PostEvent): Likewise.
Index: jvmti.cc
===================================================================
--- jvmti.cc (revision 121442)
+++ jvmti.cc (working copy)
@@ -27,7 +27,6 @@
#include <java/lang/Class.h>
#include <java/lang/ClassLoader.h>
-#include <java/lang/Object.h>
#include <java/lang/OutOfMemoryError.h>
#include <java/lang/Thread.h>
#include <java/lang/ThreadGroup.h>
@@ -37,6 +36,8 @@
#include <java/lang/reflect/Modifier.h>
#include <java/util/Collection.h>
#include <java/util/HashMap.h>
+#include <java/util/concurrent/locks/Lock.h>
+#include <java/util/concurrent/locks/ReentrantReadWriteLock.h>
#include <java/net/URL.h>
static void check_enabled_events (void);
@@ -103,7 +104,8 @@
struct jvmti_env_list *next;
};
static struct jvmti_env_list *_jvmtiEnvironments = NULL;
-static java::lang::Object *_envListLock = NULL;
+static java::util::concurrent::locks::
+ReentrantReadWriteLock *_envListLock = NULL;
#define FOREACH_ENVIRONMENT(Ele) \
for (Ele = _jvmtiEnvironments; Ele != NULL; Ele = Ele->next)
@@ -896,7 +898,7 @@
return JVMTI_ERROR_INVALID_ENVIRONMENT;
else
{
- JvSynchronize dummy (_envListLock);
+ _envListLock->writeLock ()->lock ();
if (_jvmtiEnvironments->env == env)
{
struct jvmti_env_list *next = _jvmtiEnvironments->next;
@@ -909,12 +911,16 @@
while (e->next != NULL && e->next->env != env)
e = e->next;
if (e->next == NULL)
- return JVMTI_ERROR_INVALID_ENVIRONMENT;
+ {
+ _envListLock->writeLock ()->unlock ();
+ return JVMTI_ERROR_INVALID_ENVIRONMENT;
+ }
struct jvmti_env_list *next = e->next->next;
_Jv_Free (e->next);
e->next = next;
}
+ _envListLock->writeLock ()->unlock ();
}
_Jv_Free (env);
@@ -1215,18 +1221,24 @@
int index = EVENT_INDEX (type); // safe since caller checks this
- JvSynchronize dummy (_envListLock);
- struct jvmti_env_list *e;
- FOREACH_ENVIRONMENT (e)
+ if (_jvmtiEnvironments != NULL)
{
- char *addr
- = reinterpret_cast<char *> (&e->env->callbacks) + offset;
- void **callback = reinterpret_cast<void **> (addr);
- if (e->env->enabled[index] && *callback != NULL)
+ _envListLock->readLock ()->lock ();
+ struct jvmti_env_list *e;
+ FOREACH_ENVIRONMENT (e)
{
- *enabled = true;
- return;
+ char *addr
+ = reinterpret_cast<char *> (&e->env->callbacks) + offset;
+ void **callback = reinterpret_cast<void **> (addr);
+ if (e->env->enabled[index] && *callback != NULL)
+ {
+ *enabled = true;
+ _envListLock->readLock ()->unlock ();
+ return;
+ }
}
+
+ _envListLock->readLock ()->unlock ();
}
*enabled = false;
@@ -1739,25 +1751,23 @@
_Jv_JVMTIEnv *env
= (_Jv_JVMTIEnv *) _Jv_MallocUnchecked (sizeof (_Jv_JVMTIEnv));
env->p = &_Jv_JVMTI_Interface;
+ struct jvmti_env_list *element
+ = (struct jvmti_env_list *) _Jv_MallocUnchecked (sizeof (struct jvmti_env_list));
+ element->env = env;
+ element->next = NULL;
- {
- JvSynchronize dummy (_envListLock);
- struct jvmti_env_list *element
- = (struct jvmti_env_list *) _Jv_MallocUnchecked (sizeof (struct jvmti_env_list));
- element->env = env;
- element->next = NULL;
+ _envListLock->writeLock ()->lock ();
+ if (_jvmtiEnvironments == NULL)
+ _jvmtiEnvironments = element;
+ else
+ {
+ struct jvmti_env_list *e;
+ for (e = _jvmtiEnvironments; e->next != NULL; e = e->next)
+ ;
+ e->next = element;
+ }
+ _envListLock->writeLock ()->unlock ();
- if (_jvmtiEnvironments == NULL)
- _jvmtiEnvironments = element;
- else
- {
- struct jvmti_env_list *e;
- for (e = _jvmtiEnvironments; e->next != NULL; e = e->next)
- ;
- e->next = element;
- }
- }
-
/* Mark JVMTI active. This is used to force the interpreter
to use either debugging or non-debugging code. Once JVMTI
has been enabled, the non-debug interpreter cannot be used. */
@@ -1769,7 +1779,8 @@
_Jv_JVMTI_Init ()
{
_jvmtiEnvironments = NULL;
- _envListLock = new java::lang::Object ();
+ _envListLock
+ = new java::util::concurrent::locks::ReentrantReadWriteLock ();
// No environments, so this should set all JVMTI:: members to false
check_enabled_events ();
@@ -2133,7 +2144,7 @@
va_list args;
va_start (args, event_thread);
- JvSynchronize dummy (_envListLock);
+ _envListLock->readLock ()->lock ();
struct jvmti_env_list *e;
FOREACH_ENVIRONMENT (e)
{
@@ -2149,6 +2160,6 @@
post_event (e->env, type, event_thread, args);
}
}
-
+ _envListLock->readLock ()->unlock ();
va_end (args);
}