Re: gnu_java_awt_peer_gtk_GdkFontPeer.c (initStaticState): missing NewGlobalRef?

2005-12-03 Thread Mark Wielaard
Hi Christian,

I finally read the paper and took a look at the list. But you already
fixed all the obvious things related to class fields. So the remaining
things left seem to be jmethodIDs that are cached, but where we don't
have a global ref to the class. Like the following:

 B   gnu_java_awt_peer_gtk_GtkWindowPeer.c   272 (jmethodID)
 B   gnu_java_awt_peer_gtk_GtkWindowPeer.c   275 (jmethodID)
 B   gnu_java_awt_peer_gtk_GtkWindowPeer.c   279 (jmethodID)
 B   gnu_java_awt_peer_gtk_GtkWindowPeer.c   282 (jmethodID)
 B   gnu_java_awt_peer_gtk_GtkWindowPeer.c   286 (jmethodID)
 B   gnu_java_awt_peer_gtk_GtkWindowPeer.c   289 (jmethodID)

But are these really a problem?
For GNU Classpath they are probably not a problem since these classes
and naitve libraries are loaded by the bootstrap class loader so will
never be unloaded. But is it even a real problem when the class and the
native library are loaded by a user defined class loader?

It seems that both the class reference can only disappear (and making
the mathod id cache invalid) when the class loader is garbage collected.
But in that case the native library will also be unloaded. So any method
field ID caching will be redone when the class and native library are
loaded again.

What do you think?

Cheers,

Mark


signature.asc
Description: This is a digitally signed message part
___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


[cp-patches] FYI: Was: Re: gnu_java_awt_peer_gtk_GdkFontPeer.c (initStaticState): missing NewGlobalRef?

2005-11-15 Thread Christian Thalinger
Ooops!  Almost forgot to commit this.

TWISTI


2005-11-15  Christian Thalinger  [EMAIL PROTECTED]

* native/jni/java-lang/java_lang_VMDouble.c (initIDs): Register 
clsDouble as global ref.
* native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkToolkit.c 
(gtkInit): Register gtkgenericpeer as global ref.


Index: native/jni/java-lang/java_lang_VMDouble.c
===
RCS file: 
/cvsroot/classpath/classpath/native/jni/java-lang/java_lang_VMDouble.c,v
retrieving revision 1.11
diff -u -3 -p -r1.11 java_lang_VMDouble.c
--- native/jni/java-lang/java_lang_VMDouble.c   24 Aug 2005 14:18:52 -  
1.11
+++ native/jni/java-lang/java_lang_VMDouble.c   15 Nov 2005 20:09:25 -
@@ -71,6 +71,11 @@ Java_java_lang_VMDouble_initIDs (JNIEnv 
 {
   DBG (unable to get class java.lang.Double\n) return;
 }
+  clsDouble = (*env)-NewGlobalRef(env, clsDouble);
+  if (clsDouble == NULL)
+{
+  DBG (unable to register class java.lang.Double as global ref\n) return;
+}
   isNaNID = (*env)-GetStaticMethodID (env, clsDouble, isNaN, (D)Z);
   if (isNaNID == NULL)
 {
[EMAIL PROTECTED]:~/src/cacao/classpath/classpath$ cvs di 
native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkToolkit.c 
Index: native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkToolkit.c
===
RCS file: 
/cvsroot/classpath/classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkToolkit.c,v
retrieving revision 1.24
diff -u -3 -p -r1.24 gnu_java_awt_peer_gtk_GtkToolkit.c
--- native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkToolkit.c  24 Sep 2005 
21:01:07 -  1.24
+++ native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkToolkit.c  15 Nov 2005 
20:10:59 -
@@ -135,6 +135,8 @@ Java_gnu_java_awt_peer_gtk_GtkToolkit_gt
 
   gtkgenericpeer = (*env)-FindClass(env, 
gnu/java/awt/peer/gtk/GtkGenericPeer);
 
+  gtkgenericpeer = (*env)-NewGlobalRef(env, gtkgenericpeer);
+
   printCurrentThreadID = (*env)-GetStaticMethodID (env, gtkgenericpeer,
 printCurrentThread, 
()V);
  




___
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches


Re: gnu_java_awt_peer_gtk_GdkFontPeer.c (initStaticState): missing NewGlobalRef?

2005-11-15 Thread Christian Thalinger
On Tue, Nov 08, 2005 at 07:09:43PM +0100, Mark Wielaard wrote:
 code analyzer for this. Please let us know when the paper is published.

Here is the first version of the paper:

http://www.complang.tuwien.ac.at/cacaojvm//papers/space.dvi.gz

TWISTI


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


Re: gnu_java_awt_peer_gtk_GdkFontPeer.c (initStaticState): missing NewGlobalRef?

2005-11-15 Thread Christian Thalinger
On Thu, 2005-11-10 at 20:57 +0100, Mark Wielaard wrote:
 These indeed look like genuine bugs. Thanks for finding them. Could you
 post the patch plus a ChangeLog entry to classpath-patches and commit
 these?

Took some time, but done.

TWISTI



___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


Re: gnu_java_awt_peer_gtk_GdkFontPeer.c (initStaticState): missing NewGlobalRef?

2005-11-10 Thread Christian Thalinger
On Tue, 2005-11-08 at 16:42 +0100, Christian Thalinger wrote:
 We are currently developing a JNI source code analyzer, which scans for
 missing NewGlobalRef calls (this is for popl06).  And it seems that it
 has found a bug in GNU classpath's gtk peers.

Ok, here are the 2 missing jclass bugs we've found:

Index: native/jni/java-lang/java_lang_VMDouble.c
===
RCS file: 
/cvsroot/classpath/classpath/native/jni/java-lang/java_lang_VMDouble.c,v
retrieving revision 1.11
diff -u -3 -p -r1.11 java_lang_VMDouble.c
--- native/jni/java-lang/java_lang_VMDouble.c   24 Aug 2005 14:18:52 -  
1.11
+++ native/jni/java-lang/java_lang_VMDouble.c   10 Nov 2005 19:34:14 -
@@ -71,6 +71,11 @@ Java_java_lang_VMDouble_initIDs (JNIEnv 
 {
   DBG (unable to get class java.lang.Double\n) return;
 }
+  clsDouble = (*env)-NewGlobalRef(env, clsDouble);
+  if (clsDouble == NULL)
+{
+  DBG (unable to register class java.lang.Double as global ref\n) return;
+}
   isNaNID = (*env)-GetStaticMethodID (env, clsDouble, isNaN, (D)Z);
   if (isNaNID == NULL)
 {
Index: native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkToolkit.c
===
RCS file: 
/cvsroot/classpath/classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkToolkit.c,v
retrieving revision 1.24
diff -u -3 -p -r1.24 gnu_java_awt_peer_gtk_GtkToolkit.c
--- native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkToolkit.c  24 Sep 2005 
21:01:07 -  1.24
+++ native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkToolkit.c  10 Nov 2005 
19:34:14 -
@@ -135,6 +135,8 @@ Java_gnu_java_awt_peer_gtk_GtkToolkit_gt
 
   gtkgenericpeer = (*env)-FindClass(env, 
gnu/java/awt/peer/gtk/GtkGenericPeer);
 
+  gtkgenericpeer = (*env)-NewGlobalRef(env, gtkgenericpeer);
+
   printCurrentThreadID = (*env)-GetStaticMethodID (env, gtkgenericpeer,
 printCurrentThread, 
()V);
  

There are a lot more warnings and potential bugs concerning jmethod,
jfieldID, etc.  Should i post a list of the warnings i think they may be
a bug (some of them are marked as strange)?

TWISTI



___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


Re: gnu_java_awt_peer_gtk_GdkFontPeer.c (initStaticState): missing NewGlobalRef?

2005-11-10 Thread Mark Wielaard
Hi Christian,

On Thu, 2005-11-10 at 20:38 +0100, Christian Thalinger wrote:
 Ok, here are the 2 missing jclass bugs we've found:

These indeed look like genuine bugs. Thanks for finding them. Could you
post the patch plus a ChangeLog entry to classpath-patches and commit
these?

 There are a lot more warnings and potential bugs concerning jmethod,
 jfieldID, etc.  Should i post a list of the warnings i think they may be
 a bug (some of them are marked as strange)?

Yes, please do post a list if you have one and we can go through them
and see if they might indeed cause bugs/unexpected behavior. Even if
they are not really problematic in this context it would be instructive
what kind of potential issues there are.

Thanks,

Mark

-- 
Escape the Java Trap with GNU Classpath!
http://www.gnu.org/philosophy/java-trap.html

Join the community at http://planet.classpath.org/


signature.asc
Description: This is a digitally signed message part
___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


Re: gnu_java_awt_peer_gtk_GdkFontPeer.c (initStaticState): missing NewGlobalRef?

2005-11-10 Thread Christian Thalinger
On Thu, 2005-11-10 at 20:57 +0100, Mark Wielaard wrote:
 Yes, please do post a list if you have one and we can go through them
 and see if they might indeed cause bugs/unexpected behavior. Even if
 they are not really problematic in this context it would be instructive
 what kind of potential issues there are.

Here is the list of the (potential) bugs.  Those marked with an S are
kinda strange and maybe not really a bug.  This list also includes the
ones i've already reported.

TWISTI


B   java_lang_VMDouble.c69  (jclass)
B   java_lang_VMDouble.c74  (jmethodID)
B   gnu_java_nio_channels_FileChannelImpl.c 131 (jfieldID)
B   gnu_java_nio_charset_iconv_IconvDecoder.c   82  (jfieldID)
B   gnu_java_nio_charset_iconv_IconvDecoder.c   84  (jfieldID)
B   gnu_java_nio_charset_iconv_IconvEncoder.c   82  (jfieldID)
B   gnu_java_nio_charset_iconv_IconvEncoder.c   84  (jfieldID)
B   gnu_java_awt_peer_gtk_GdkGraphics2D.c   68  (jmethodID)
B   gnu_java_awt_peer_gtk_GdkFontPeer.c 65  (jclass)
B   gnu_java_awt_peer_gtk_GdkFontPeer.c 68  (jmethodID)
B   gnu_java_awt_peer_gtk_GdkGraphics.c 54  (jmethodID)
B   gnu_java_awt_peer_gtk_GdkPixbufDecoder.c287 (jmethodID)
B   gnu_java_awt_peer_gtk_GdkPixbufDecoder.c291 (jmethodID)
B   gnu_java_awt_peer_gtk_GdkPixbufDecoder.c295 (jmethodID)
B   gnu_java_awt_peer_gtk_GdkPixbufDecoder.c303 (jmethodID)
B   gnu_java_awt_peer_gtk_GtkButtonPeer.c   52  (jmethodID)
B   gnu_java_awt_peer_gtk_GtkCheckboxPeer.c 53  (jmethodID)
B   gnu_java_awt_peer_gtk_GtkChoicePeer.c   52  (jmethodID)
B S gnu_java_awt_peer_gtk_GtkClipboard.c96  (jclass) NOT FOUND
B   gnu_java_awt_peer_gtk_GtkClipboard.c97  (jmethodID)
B   gnu_java_awt_peer_gtk_GtkClipboard.c362 (jmethodID)
B   gnu_java_awt_peer_gtk_GtkClipboard.c369 (jmethodID)
B   gnu_java_awt_peer_gtk_GtkClipboard.c375 (jmethodID)
B   gnu_java_awt_peer_gtk_GtkClipboard.c382 (jmethodID)
B   gnu_java_awt_peer_gtk_GtkComponentPeer.c90  (jmethodID)
B   gnu_java_awt_peer_gtk_GtkComponentPeer.c93  (jmethodID)
B   gnu_java_awt_peer_gtk_GtkComponentPeer.c96  (jmethodID)
B   gnu_java_awt_peer_gtk_GtkComponentPeer.c99  (jmethodID)
B   gnu_java_awt_peer_gtk_GtkFileDialogPeer.c   64  (jmethodID)
B   gnu_java_awt_peer_gtk_GtkFileDialogPeer.c   69  (jmethodID)
B   gnu_java_awt_peer_gtk_GtkFileDialogPeer.c   74  (jmethodID)
B   gnu_java_awt_peer_gtk_GtkFileDialogPeer.c   80  (jmethodID)
B   gnu_java_awt_peer_gtk_GtkListPeer.c 51  (jmethodID)
B   gnu_java_awt_peer_gtk_GtkMenuItemPeer.c 53  (jmethodID)
B   gnu_java_awt_peer_gtk_GtkScrollbarPeer.c60  (jmethodID)
B S gnu_java_awt_peer_gtk_GtkSelection.c162 (jmethodID)
B S gnu_java_awt_peer_gtk_GtkSelection.c219 (jmethodID)
B S gnu_java_awt_peer_gtk_GtkSelection.c269 (jmethodID)
B S gnu_java_awt_peer_gtk_GtkSelection.c353 (jmethodID)
B S gnu_java_awt_peer_gtk_GtkSelection.c417 (jmethodID)
B   gnu_java_awt_peer_gtk_GtkTextFieldPeer.c58  (jmethodID)
B   gnu_java_awt_peer_gtk_GtkToolkit.c  136 (jclass)
B   gnu_java_awt_peer_gtk_GtkToolkit.c  138 (jmethodID)
B   gnu_java_awt_peer_gtk_GtkWindowPeer.c   272 (jmethodID)
B   gnu_java_awt_peer_gtk_GtkWindowPeer.c   275 (jmethodID)
B   gnu_java_awt_peer_gtk_GtkWindowPeer.c   279 (jmethodID)
B   gnu_java_awt_peer_gtk_GtkWindowPeer.c   282 (jmethodID)
B   gnu_java_awt_peer_gtk_GtkWindowPeer.c   286 (jmethodID)
B   gnu_java_awt_peer_gtk_GtkWindowPeer.c   289 (jmethodID)




___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


[cp-patches] FYI: Was: gnu_java_awt_peer_gtk_GdkFontPeer.c (initStaticState): missing NewGlobalRef?

2005-11-09 Thread Christian Thalinger
Hi!

As discussed on classpath, this one is commited.  Maybe more to come...

TWISTI


2005-11-09  Christian Thalinger  [EMAIL PROTECTED]

* native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkFontPeer.c 
(initStaticState): Register global variable glyphVector_class 
as global reference.


Index: native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkFontPeer.c
===
RCS file: 
/cvsroot/classpath/classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkFontPeer.c,v
retrieving revision 1.10
diff -u -3 -p -r1.10 gnu_java_awt_peer_gtk_GdkFontPeer.c
--- native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkFontPeer.c 19 Sep 2005 
05:47:09 -  1.10
+++ native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkFontPeer.c 9 Nov 2005 
09:57:14 -
@@ -65,6 +65,9 @@ Java_gnu_java_awt_peer_gtk_GdkFontPeer_i
   glyphVector_class = (*env)-FindClass 
 (env, gnu/java/awt/peer/gtk/GdkGlyphVector);
 
+  glyphVector_class = (*env)-NewGlobalRef
+(env, glyphVector_class);
+
   glyphVector_ctor = (*env)-GetMethodID 
 (env, glyphVector_class, init, 
  ([D[ILjava/awt/Font;Ljava/awt/font/FontRenderContext;)V);




___
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches


Re: gnu_java_awt_peer_gtk_GdkFontPeer.c (initStaticState): missing NewGlobalRef?

2005-11-09 Thread Christian Thalinger
On Tue, 2005-11-08 at 19:09 +0100, Mark Wielaard wrote:
 Yes, nice catch. Now that I have seen this I am surprised we don't have
 more bugs like this one. it is easy to miss. Cool to know you have a
 code analyzer for this. Please let us know when the paper is published.

We currently have only analyzed the gtk-peer code, since our tool still
has some minor bugs.  But today we should be able to analyze the whole
GNU classpath jni code.  We'll see how many bugs we find :-)

The paper should be finished tomorrow.

TWISTI



___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


Re: gnu_java_awt_peer_gtk_GdkFontPeer.c (initStaticState): missing NewGlobalRef?

2005-11-09 Thread Christian Thalinger
On Tue, 2005-11-08 at 18:32 -0700, Tom Tromey wrote:
  Twisti == Christian Thalinger [EMAIL PROTECTED] writes:
 Perhaps when it is ready we could integrate it into the build, so
 that errors here are unavoidable?

Uhm... that will be difficult since the tool is an enhancement of GCC.
It's just another pass to print out some (conservative) warnings about
potential missing NewGlobalRef calls.

TWISTI



___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


gnu_java_awt_peer_gtk_GdkFontPeer.c (initStaticState): missing NewGlobalRef?

2005-11-08 Thread Christian Thalinger
Hi!

We are currently developing a JNI source code analyzer, which scans for
missing NewGlobalRef calls (this is for popl06).  And it seems that it
has found a bug in GNU classpath's gtk peers.

In Java_gnu_java_awt_peer_gtk_GdkFontPeer_initStaticState the global
variable glyphVector_class is not registered and is used afterwards in
getGlyphVector to instantiate a new object.

I think this should be something like:

Index: gnu_java_awt_peer_gtk_GdkFontPeer.c
===
RCS file:
/cvsroot/classpath/classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GdkFontPeer.c,v
retrieving revision 1.10
diff -u -3 -p -r1.10 gnu_java_awt_peer_gtk_GdkFontPeer.c
--- gnu_java_awt_peer_gtk_GdkFontPeer.c 19 Sep 2005 05:47:09 -
1.10
+++ gnu_java_awt_peer_gtk_GdkFontPeer.c 8 Nov 2005 15:37:39 -
@@ -65,6 +65,9 @@ Java_gnu_java_awt_peer_gtk_GdkFontPeer_i
   glyphVector_class = (*env)-FindClass 
 (env, gnu/java/awt/peer/gtk/GdkGlyphVector);
 
+  glyphVector_class = (*env)-NewGlobalRef
+(env, glyphVector_class);
+
   glyphVector_ctor = (*env)-GetMethodID 
 (env, glyphVector_class, init, 
  ([D[ILjava/awt/Font;Ljava/awt/font/FontRenderContext;)V);

Comments?

TWISTI


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


Re: gnu_java_awt_peer_gtk_GdkFontPeer.c (initStaticState): missing NewGlobalRef?

2005-11-08 Thread Mark Wielaard
Hi Christian,

On Tue, 2005-11-08 at 16:42 +0100, Christian Thalinger wrote:
 We are currently developing a JNI source code analyzer, which scans for
 missing NewGlobalRef calls (this is for popl06).  And it seems that it
 has found a bug in GNU classpath's gtk peers.
 
 In Java_gnu_java_awt_peer_gtk_GdkFontPeer_initStaticState the global
 variable glyphVector_class is not registered and is used afterwards in
 getGlyphVector to instantiate a new object.

Yes, nice catch. Now that I have seen this I am surprised we don't have
more bugs like this one. it is easy to miss. Cool to know you have a
code analyzer for this. Please let us know when the paper is published.

Could you post this patch plus ChangeLog entry to classpath-patches and
commit it?

Thanks,

Mark

-- 
Escape the Java Trap with GNU Classpath!
http://www.gnu.org/philosophy/java-trap.html

Join the community at http://planet.classpath.org/


signature.asc
Description: This is a digitally signed message part
___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath


Re: gnu_java_awt_peer_gtk_GdkFontPeer.c (initStaticState): missing NewGlobalRef?

2005-11-08 Thread Tom Tromey
 Twisti == Christian Thalinger [EMAIL PROTECTED] writes:

Twisti We are currently developing a JNI source code analyzer, which scans for
Twisti missing NewGlobalRef calls (this is for popl06).  And it seems that it
Twisti has found a bug in GNU classpath's gtk peers.

Very cool.

Perhaps when it is ready we could integrate it into the build, so
that errors here are unavoidable?

Tom


___
Classpath mailing list
Classpath@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath