Hi, Alexander,
the fix looks fine in general. A few comments:
1. A few comments in gtk2_interface.c about gtk versions would be fine.
2. lock()/unlock() pattern is usually used with try - finally. Could you
modify the example in GThreadHelper, please? We don't expect C code to
use try - finally, but when we use GThreadHelper from FX, we'll do.
3. I would keep g_thread_get_initalized() check, when it's available.
That is, for versions < 2.20 we only use our own flag (GThreadHelper),
for 2.21+ use both our flag and also consult with glib. BTW, is
g_thread_get_initialized() really deprecated? If that is the case, it
doesn't worth to use it, but rely on GThreadHelper only.
Thanks,
Artem
On 9/24/2013 2:40 PM, Alexander Zvegintsev wrote:
Hello,
Please review the fix for the issue:
https://bugs.openjdk.java.net/browse/JDK-7059886
The webrev is available here:
http://cr.openjdk.java.net/~serb/alexz/7059886/webrev.00/
For old versions of GLib (< 2.24) calling g_thread_init () [1] multiple
times will crash an application.
There are two ways to find out if g_thread_init() has been called:
g_thread_supported ()[2] and g_thread_get_initialized ()[3]
g_thread_supported () is a macro, so we cannot load it with dlsym
g_thread_get_initialized () was introduced in 2.20, but we have to
support versions < 2.20
Currently in JDK we have an internal flag which protects us from such
multiple calls, but at least we
have Java FX which does not have access to this flag.
The idea of the fix it to make single entry point for everyone who is
about to call g_thread_init () to
check if it has been called.
Should we bring back g_thread_get_initialized () check for GLib >= 2.20
for safety?
[1]
https://developer.gnome.org/glib/stable/glib-Deprecated-Thread-APIs.html#g-thread-init
[2]
https://developer.gnome.org/glib/stable/glib-Deprecated-Thread-APIs.html#g-thread-supported
[3]
https://developer.gnome.org/glib/stable/glib-Deprecated-Thread-APIs.html#g-thread-get-initialized
