Thank you very much, Damjan!
I published your latest patch at:
http://cr.openjdk.java.net/~anthony/7-43-GTKFileDialog-6913179.4/
However, it still contains incorrectly formatted lines which makes the
code unreadable. You might want to explore the Sdiffs in the webrev to
notice that (eg., UNIXToolkit.java, awt_UNIXToolkit.c,
sun_awt_X11_GtkFileDialogPeer.c, etc.). Would you or Costantino please
correct that?
The fix looks fine in overall. Peter might provide some comments after
reviewing that as well.
PS. The nimbusL&F issue does not seem reproducible locally. Perhaps
you're using an outdated repository, or your building environment has
slightly changed.
--
best regards,
Anthony
On 03/11/2010 01:09 PM, Damjan Jovanovic wrote:
On Wed, Mar 10, 2010 at 11:35 PM, Costantino Cerbo <[email protected]> wrote:
Hallo,
Hi
the only change in this new patch is the include of <string.h> and and
<jni_util.h>.
The answers for Damjan:
1) Using gdk_threads_enter() and gdk_threads_leave() in
Java_sun_awt_X11_GtkFileDialogPeer_quit(..) freezes the dialog on
close.
Sun, your latest awt tree still doesn't compile for me, I had to hack
it to not call a method that doesn't exist so it does compile:
diff -r d6d2de6ee2d1
src/share/classes/javax/swing/plaf/nimbus/NimbusLookAndFeel.java
--- a/src/share/classes/javax/swing/plaf/nimbus/NimbusLookAndFeel.java
Fri Feb 19 15:13:37 2010 -0800
+++ b/src/share/classes/javax/swing/plaf/nimbus/NimbusLookAndFeel.java
Thu Mar 11 11:33:29 2010 +0200
@@ -288,7 +288,7 @@
"JComponent.sizeVariant" == eName) {
JComponent c = (JComponent) ev.getSource();
- defaults.clearOverridesCache(c);
+ //defaults.clearOverridesCache(c);
return true;
}
Now onto your patch.
You probably didn't listen to my warning about not calling
gdk_threads_enter when Java_sun_awt_X11_GtkFileDialogPeer_quit is
called from handle_response (because GTK itself holds that GDK lock
during the entire handle_response invocation, and that lock is not
reentrant). Hence you got the freeze.
My tests show the way I described does work, and the patch that uses
it correctly and doesn't freeze is attached.
2) This patch is tested with the last code in the OpenJDK repository
and compiles correctly.
I think, we all have done a good job and now it's time to put the
patch in the OpenJDK.
With my patch and some heavier testing, I managed to get an assertion failure:
java: ../../src/xcb_io.c:176: process_responses: Assertion `!(req &&
current_request && !(((long) (req->sequence) - (long)
(current_request)) <= 0))' failed.
Aborted
I couldn't reproduce it but I'll keep trying.
There's probably still some place where we're not locking properly.
Best Regards,
Costantino
Regards
Damjan
2010/3/10 Costantino Cerbo <[email protected]>:
Hello Damjan,
2010/3/10 Damjan Jovanovic <[email protected]>:
You should probably include the real header files here, like string.h
and jni_util.h, instead of declaring those functions yourself.
You're right. I will do so.
Should we put the GDK lock statements around those 3 functions? But if
we should, make sure they aren't taken recursively: when called from
Java the GDK lock isn't taken and we must, but when called from the
handle_response callback function, GTK already holds that lock and we
must not.
I don't know. I'll try later and let you know.
You've still got some C++ comments:
$ grep '//' b6913179.patch
They are comments in Java classes ;-)
Also the patch doesn't even compile from me, fpulling the awt forest
to the latest and trying again...
Yesterday I could compile without problems.
When I go home later, I check out the last code in the repository and try again.
Thanks for your comments and best regards,
Costantino
