Hi Alexander, A few comments on the GThreadHelper class:
1. I think the static methods in the GThreadHelper class should be public, because they are intended to be called by external code. Note that all public symbols should have a javadoc.
2. Why do we need both isInitializationNeeded() and initFinished()? Can the isInitializationNeeded() act (and be named) like the AtomicBoolean.compareAndSet/getAndSet() methods, in that it would automatically set the initialized flag to true? Or do you see a use case when a code may want to check that GTK isn't initialized yet and still not initialize it afterwards? What could this be needed for?
3. Since the flag has to be queried/modified under the LOCK only, it doesn't need to be volatile.
4. In Javadocs, the first sentence usually summarizes what a method does. Locking requirements should be stated either at the end of the first sentence, or in a subsequent sentence.
-- best regards, Anthony On 09/24/2013 02: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
