Hi Alexander,
How about getAndSetInitializationNeededFlag ?
74 * If it returns {@code false} user must call g_thread_init() on his
own.
"on their own"? :) Yet, it might be better phrased as:
"A return value of {@code false} indicates that the calling code should
call the g_thread_init() function in Glib before releasing the lock."
src/solaris/native/sun/awt/gtk2_interface.c
837 //According the GTK documentation, gdk_threads_init() should be
838 //called before gtk_init() or gtk_init_check()
839 fp_gdk_threads_init();
Interesting. I see we only specify we want user code to call
g_thread_init(), while here we also call gdk_threads_init() and use the
same flag to decide whether to call it or not. Is this OK? What about
other clients (FX)?
--
best regards,
Anthony
On 09/27/2013 06:02 PM, Alexander Zvegintsev wrote:
Hi Anthony, Artem,
here is a new version of webrev with suggested improvements:
http://cr.openjdk.java.net/~serb/alexz/7059886/webrev.01/
Name of a function in GThreadHelper is not ideal, but I can't think out
any better.
We don't expect C code to use try - finally, but when we use
GThreadHelper from FX, we'll do.
In FX we will use it from native too.
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.
I think it worth to use it. It is deprerecated due to:
|g_thread_init| has been deprecated since version 2.32 and should not
be used in newly-written code. The GLib threading system is
automatically initialized at the start of your program.
hence g_thread_get_initialized() returns always true on a modern GLib
versions.
Thanks,
Alexander.
On 09/24/2013 04:12 PM, Anthony Petrov wrote:
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
