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

[Anton Litvinov]

Hello Anthony,

Thank you very much for the review of this request. Yes, you understood 
the description of the bug correctly. After several discussions with 
Artem I also was recommended to redesign J2DXErrHandler() as a Java 
synthetic handler and to exploit 
"sun.awt.X11.XTookit.WITH_XERROR_HANDLER" through JNI from all the 
places in native code which currently set/unset J2DXErrHandler(). I 
managed to implement this solution, but it led to a crash with 
IndexOutOfBoundsException not in places of 
"src/solaris/native/sun/awt/awt_GraphicsEnv.c", 
"src/solaris/native/sun/java2d/x11/X11SurfaceData.c" files, where 
substitution for the Java synthetic handler was made, but in some other 
AWT Java files handling fonts. I was not able to resolve this crash and 
came to a conclusion that, perhaps, static initialization of 
"sun.awt.X11.XToolkit" class in

void TryInitMITShm(JNIEnv *env, jint *shmExt, jint *shmPixmaps)

function of "src/solaris/native/sun/awt/awt_GraphicsEnv.c", which was 
initiated by the code from the fix, should not have happened so early. 
Probably, some mistake was done in implementation of the fix involving 
JNI. Then I will try to return to the solution with synthetic handlers 
from XToolkit.

Thank you,
Anton

On 1/9/2013 8:08 PM, 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