<AWT Dev> [8] Review request 7059886: 6 JCK manual awt/Desktop tests fail with GTKLookAndFeel - GTK intialization issue

[Anthony Petrov]

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
>>>
>>>
>>>
>