Re: gnu_java_awt_peer_gtk_GdkFontPeer.c (initStaticState): missing NewGlobalRef?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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?
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