<AWT Dev> [OpenJDK 2D-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 08:14:47 PST 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/awt-dev/attachments/20130130/0ecf0ab5/attachment.html
More information about the awt-dev
mailing list