<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    Hello Anthony,<br>
    <br>
    Could you, please, review a second version of the fix, which is
    based on an idea of reusing the existing AWT native global error
    handler from Java 2D native code.<br>
    <br>
    Webrev: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~alitvinov/8005607/webrev.01">http://cr.openjdk.java.net/~alitvinov/8005607/webrev.01</a><br>
    <br>
    The fix consists of the following parts:<br>
    <ol>
      <li>Migration of all X error handling code from XToolkit to a new
        XErrorHandlerUtil class for resolution of interdependency
        between a static initialization block of XToolkit and a block
        initializing java.awt.GraphicsEnvironment singleton. Such
        dependency is created by new calls to XToolkit static methods
        from "src/solaris/native/sun/awt/awt_GraphicsEnv.c",
        "src/solaris/native/sun/java2d/x11/X11SurfaceData.c" files.</li>
      <li>Substitution of XToolkit.WITH_XERROR_HANDLER,
        XToolkit.RESTORE_XERROR_HANDLER ... for corresponding methods,
        fields of XErrorHandlerUtil class in all places of JDK source
        code, where they were used.</li>
      <li>Substitution of all found native X error handlers which are
        set in native code (awt_GraphicsEnv.c, X11SurfaceData.c,
        GLXSurfaceData.c) for new synthetic Java error handlers.</li>
      <li>Removal of X error handling code used by the native error
        handlers from "solaris/native/sun/awt/awt_util.c"
        "solaris/native/sun/awt/awt_util.h" files.</li>
    </ol>
    Thank you,<br>
    Anton<br>
    <br>
    On 1/11/2013 3:45 PM, Anthony Petrov wrote:
    <blockquote cite="mid:50EFFB4F.4090405@oracle.com" type="cite">I'm
      not Jim, but as I indicated earlier my opinion is that the easiest
      way to fix this is to install the existing J2DXErrHandler() only
      once. That is, it is the second option listed by you. Of course,
      the J2DXErrHandler needs to be updated as well to detect whether
      2D code wants to use it at the moment or it must simply delegate
      to the previous handler (i.e. where the code currently
      installs/uninstalls the handler, it must instead set a global
      boolean flag or something.)
      <br>
      <br>
      While the first option (reusing the existing AWT machinery) is an
      interesting idea in general, I think it is complex and would
      require too much additional testing and bring an unjustified risk
      to the solution for such a basic problem.
      <br>
      <br>
      --
      <br>
      best regards,
      <br>
      Anthony
      <br>
      <br>
      On 1/11/2013 14:44, Anton Litvinov wrote:
      <br>
      <blockquote type="cite">Hello Jim,
        <br>
        <br>
        Thank you very much for the review and provision of a new idea
        of a solution. Elimination of the logic, which sets/unsets
        J2DXErrHandler() for each call "XShmAttach(awt_display,
        &shminfo))" should effectively resolve the issue, but only
        in case if all other native error handlers, which were set by
        the system function "XSetErrorHandler()" in JDK or in any
        external library, observe the rule of relaying of all events,
        which are not relative to them, to the previously saved error
        handlers. Otherwise an error generated during "XShmAttach"
        function call will not be handled by J2DXErrHandler().
        <br>
        <br>
        Could you answer the following question. By setting
        J2DXErrHandler() only once and forever do you mean usage of AWT
        global event handler "static int ToolkitErrorHandler(Display *
        dpy, XErrorEvent * event)" from
        "src/solaris/native/sun/xawt/XlibWrapper.c" with Java synthetic
        handlers or creation of another global native error handler with
        J2DXErrHandler as native synthetic handler?
        <br>
        <br>
        Thank you,
        <br>
        Anton
        <br>
        <br>
        On 1/10/2013 5:44 AM, Jim Graham wrote:
        <br>
        <blockquote type="cite">I think I'd rather see some way to
          prevent double-adding the handler in the first place as well. 
          Since it is only ever used on errors I also think it is OK to
          set it once and leave it there forever...
          <br>
          <br>
                      ...jim
          <br>
          <br>
          On 1/9/13 8:08 AM, Anthony Petrov wrote:
          <br>
          <blockquote type="cite">Hi Anton et al.,
            <br>
            <br>
            If I read the description of the bug correctly, specifically
            this part:
            <br>
            <br>
            <blockquote type="cite">The problem occurs, if another
              thread (for example, GTK thread) is
              <br>
              doing the same sort of thing concurrently. This can lead
              to the
              <br>
              following situation.
              <br>
               JVM thread: Sets J2DXErrHandler(), saves
              ANY_PREVIOUS_HANDLER as
              <br>
              previous      GTK thread: Sets some GTK_HANDLER, saves
              <br>
              J2DXErrHandler() as previous  JVM thread: Restores
              <br>
              ANY_PREVIOUS_HANDLER      GTK thread: Restores
              J2DXErrHandler()  JVM
              <br>
              thread: Sets J2DXErrHandler(), saves J2DXErrHandler() as
              previous
              <br>
            </blockquote>
            <br>
            It is obvious that at this final step 2D is in an
            inconsistent state. We
            <br>
            don't expect to replace our own error handler (and it
            shouldn't have
            <br>
            been there in the first place).
            <br>
            <br>
            I realize that the fix you propose works around this
            problem. But this
            <br>
            doesn't look like an ideal solution to me.
            <br>
            <br>
            BTW, IIRC, in JDK7 (and 6?) we decided to set the actual X11
            error
            <br>
            handler only once and never replace it. All the rest of the
            <br>
            push_handler/pop_handler logic is now located in Java code
            (see
            <br>
            XToolkit.SAVED_ERROR_HANDLER() and the surrounding logic). I
            believe
            <br>
            that we should somehow share this machinery with the 2D code
            to avoid
            <br>
            this sort of problems. Though I'm not sure if this will
            eliminate this
            <br>
            exact issue.
            <br>
            <br>
            <br>
            2D/AWT folks: any other thoughts?
            <br>
            <br>
            -- <br>
            best regards,
            <br>
            Anthony
            <br>
            <br>
            On 12/29/2012 17:44, Anton Litvinov wrote:
            <br>
            <blockquote type="cite">Hello,
              <br>
              <br>
              Please review the following fix for a bug.
              <br>
              <br>
              Bug:
              <a class="moz-txt-link-freetext" href="http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8005607">http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8005607</a>
              <br>
                       <a class="moz-txt-link-freetext" href="https://jbs.oracle.com/bugs/browse/JDK-8005607">https://jbs.oracle.com/bugs/browse/JDK-8005607</a>
              <br>
              Webrev:
              <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~alitvinov/8005607/webrev.00">http://cr.openjdk.java.net/~alitvinov/8005607/webrev.00</a>
              <br>
              <br>
              The bug consists in a crash which is caused by a stack
              overflow for
              <br>
              the reason of an infinite recursion in AWT native function
              <br>
              J2DXErrHandler() under certain circumstances on 32-bit
              Linux OS. The
              <br>
              fix is based on introduction of the logic, which detects
              indirect
              <br>
              recursive calls to J2DXErrHandler() by means of a simple
              counter, to
              <br>
              J2DXErrHandler() native function. Such a solution requires
              minimum
              <br>
              code changes, does not alter the handler's code
              significantly and
              <br>
              eliminates this bug.
              <br>
              <br>
              Adding <a class="moz-txt-link-abbreviated" href="mailto:2d-dev@openjdk.java.net">2d-dev@openjdk.java.net</a> e-mail alias to the list of
              recipients
              <br>
              of this letter, because the edited function's name is
              related to Java
              <br>
              2D area of JDK, despite of the fact that the edited file
              is located in
              <br>
              AWT directory.
              <br>
              <br>
              Thank you,
              <br>
              Anton
              <br>
            </blockquote>
          </blockquote>
        </blockquote>
      </blockquote>
    </blockquote>
  </body>
</html>