<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">Hi Anthony, Artem,<br>
      <br>
      here is a new version of webrev with suggested improvements:<br>
      <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~serb/alexz/7059886/webrev.01/">http://cr.openjdk.java.net/~serb/alexz/7059886/webrev.01/</a><br>
      <br>
      Name of a function in GThreadHelper is not ideal, but I can't
      think out any better.<br>
      <br>
      <blockquote type="cite">We don't expect C code to use try -
        finally, but when we use GThreadHelper from FX, we'll do.
      </blockquote>
      In FX we will use it from native too.<br>
      <br>
      <blockquote type="cite"> 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.
      </blockquote>
      I think it worth to use it. It is deprerecated due to: <br>
      <blockquote type="cite"><code class="literal">g_thread_init</code>
        has been deprecated since version 2.32 and should not be used in
        newly-written code. The GLib threading system is <br>
        automatically initialized at the start of your program.</blockquote>
      hence g_thread_get_initialized() returns always true on a modern
      GLib versions.<br>
      <br>
      <br>
      <pre class="moz-signature" cols="72">Thanks,

Alexander.</pre>
      On 09/24/2013 04:12 PM, Anthony Petrov wrote:<br>
    </div>
    <blockquote cite="mid:524181A9.6010903@oracle.com" type="cite">Hi
      Alexander,
      <br>
      <br>
      A few comments on the GThreadHelper class:
      <br>
      <br>
      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.
      <br>
      <br>
      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?
      <br>
      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?
      <br>
      <br>
      3. Since the flag has to be queried/modified under the LOCK only,
      it doesn't need to be volatile.
      <br>
      <br>
      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.
      <br>
      <br>
      --
      <br>
      best regards,
      <br>
      Anthony
      <br>
      <br>
      On 09/24/2013 02:40 PM, Alexander Zvegintsev wrote:
      <br>
      <blockquote type="cite">Hello,
        <br>
        <br>
        Please review the fix for the issue:
        <br>
        <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-7059886">https://bugs.openjdk.java.net/browse/JDK-7059886</a>
        <br>
        The webrev is available here:
        <br>
        <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~serb/alexz/7059886/webrev.00/">http://cr.openjdk.java.net/~serb/alexz/7059886/webrev.00/</a>
        <br>
        <br>
        For old versions of GLib (&lt; 2.24) calling g_thread_init ()
        [1] multiple
        <br>
        times will crash an application.
        <br>
        There are two ways to find out if g_thread_init() has been
        called:
        <br>
        g_thread_supported ()[2] and g_thread_get_initialized ()[3]
        <br>
        <br>
        g_thread_supported () is a macro, so we cannot load it with
        dlsym
        <br>
        g_thread_get_initialized () was introduced in 2.20, but we have
        to
        <br>
        support versions &lt; 2.20
        <br>
        <br>
        Currently in JDK we have an internal flag which protects us from
        such
        <br>
        multiple calls, but at least we
        <br>
        have Java FX which does not have access to this flag.
        <br>
        <br>
        The idea of the fix it to make single entry point for everyone
        who is
        <br>
        about to call g_thread_init () to
        <br>
        check if it has been called.
        <br>
        <br>
        Should we bring back g_thread_get_initialized () check for GLib
        &gt;= 2.20
        <br>
        for safety?
        <br>
        <br>
        [1]
        <br>
<a class="moz-txt-link-freetext" href="https://developer.gnome.org/glib/stable/glib-Deprecated-Thread-APIs.html#g-thread-init">https://developer.gnome.org/glib/stable/glib-Deprecated-Thread-APIs.html#g-thread-init</a>
        <br>
        <br>
        [2]
        <br>
<a class="moz-txt-link-freetext" href="https://developer.gnome.org/glib/stable/glib-Deprecated-Thread-APIs.html#g-thread-supported">https://developer.gnome.org/glib/stable/glib-Deprecated-Thread-APIs.html#g-thread-supported</a>
        <br>
        <br>
        [3]
        <br>
<a class="moz-txt-link-freetext" href="https://developer.gnome.org/glib/stable/glib-Deprecated-Thread-APIs.html#g-thread-get-initialized">https://developer.gnome.org/glib/stable/glib-Deprecated-Thread-APIs.html#g-thread-get-initialized</a>
        <br>
        <br>
        <br>
      </blockquote>
    </blockquote>
    <br>
  </body>
</html>