[OpenJDK 2D-Dev] <AWT Dev> [8] Review request for 8005607: Recursion in J2DXErrHandler() Causes a Stack Overflow on Linux

Anton Litvinov anton.litvinov at oracle.com
Wed Jan 30 16:14:47 UTC 2013


Hello Anthony,

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.

Webrev: http://cr.openjdk.java.net/~alitvinov/8005607/webrev.01

The fix consists of the following parts:

 1. 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.
 2. 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.
 3. 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.
 4. 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.

Thank you,
Anton

On 1/11/2013 3:45 PM, Anthony Petrov wrote:
> 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.)
>
> 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.
>
> -- 
> best regards,
> Anthony
>
> On 1/11/2013 14:44, Anton Litvinov wrote:
>> Hello Jim,
>>
>> 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().
>>
>> 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?
>>
>> Thank you,
>> Anton
>>
>> On 1/10/2013 5:44 AM, Jim Graham wrote:
>>> 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...
>>>
>>>             ...jim
>>>
>>> On 1/9/13 8:08 AM, Anthony Petrov wrote:
>>>> Hi Anton et al.,
>>>>
>>>> If I read the description of the bug correctly, specifically this 
>>>> part:
>>>>
>>>>> The problem occurs, if another thread (for example, GTK thread) is
>>>>> doing the same sort of thing concurrently. This can lead to the
>>>>> following situation.
>>>>>  JVM thread: Sets J2DXErrHandler(), saves ANY_PREVIOUS_HANDLER as
>>>>> previous      GTK thread: Sets some GTK_HANDLER, saves
>>>>> J2DXErrHandler() as previous  JVM thread: Restores
>>>>> ANY_PREVIOUS_HANDLER      GTK thread: Restores J2DXErrHandler()  JVM
>>>>> thread: Sets J2DXErrHandler(), saves J2DXErrHandler() as previous
>>>>
>>>> It is obvious that at this final step 2D is in an inconsistent 
>>>> state. We
>>>> don't expect to replace our own error handler (and it shouldn't have
>>>> been there in the first place).
>>>>
>>>> I realize that the fix you propose works around this problem. But this
>>>> doesn't look like an ideal solution to me.
>>>>
>>>> BTW, IIRC, in JDK7 (and 6?) we decided to set the actual X11 error
>>>> handler only once and never replace it. All the rest of the
>>>> push_handler/pop_handler logic is now located in Java code (see
>>>> XToolkit.SAVED_ERROR_HANDLER() and the surrounding logic). I believe
>>>> that we should somehow share this machinery with the 2D code to avoid
>>>> this sort of problems. Though I'm not sure if this will eliminate this
>>>> exact issue.
>>>>
>>>>
>>>> 2D/AWT folks: any other thoughts?
>>>>
>>>> -- 
>>>> best regards,
>>>> Anthony
>>>>
>>>> On 12/29/2012 17:44, Anton Litvinov wrote:
>>>>> Hello,
>>>>>
>>>>> Please review the following fix for a bug.
>>>>>
>>>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8005607
>>>>>          https://jbs.oracle.com/bugs/browse/JDK-8005607
>>>>> Webrev: http://cr.openjdk.java.net/~alitvinov/8005607/webrev.00
>>>>>
>>>>> The bug consists in a crash which is caused by a stack overflow for
>>>>> the reason of an infinite recursion in AWT native function
>>>>> J2DXErrHandler() under certain circumstances on 32-bit Linux OS. The
>>>>> fix is based on introduction of the logic, which detects indirect
>>>>> recursive calls to J2DXErrHandler() by means of a simple counter, to
>>>>> J2DXErrHandler() native function. Such a solution requires minimum
>>>>> code changes, does not alter the handler's code significantly and
>>>>> eliminates this bug.
>>>>>
>>>>> Adding 2d-dev at openjdk.java.net e-mail alias to the list of recipients
>>>>> of this letter, because the edited function's name is related to Java
>>>>> 2D area of JDK, despite of the fact that the edited file is 
>>>>> located in
>>>>> AWT directory.
>>>>>
>>>>> Thank you,
>>>>> Anton
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20130130/0ecf0ab5/attachment.html>


More information about the 2d-dev mailing list