<AWT Dev> [16] RFR 8232114: JVM crashed at imjpapi.dll in native code

[Anton Litvinov]

Hi Dmitry,

The fix looks well, but I have next a few comments relatively to it:

1) In the new function "AwtToolkit::InvokeInputMethodFunction(UINT, 
WPARAM, LPARAM)", which you created, the call "PostMessage(msg, wParam, 
lParam);" may fail and return "0", so in that case the next call 
"::WaitForSingleObject(m_inputMethodWaitEvent, INFINITE);" may become 
blocked forever. I suggest to handle this possible failure, for example 
in the next way:

if (PostMessage(msg, wParam, lParam)) {
     ::WaitForSingleObject(m_inputMethodWaitEvent, INFINITE);
     return m_inputMethodData;
}
return 0;

2) In "AwtToolkit::WndProc" function in the code handling the messages: 
"WM_AWT_HANDLE_NATIVE_IME_EVENT", "WM_AWT_ACTIVATEKEYBOARDLAYOUT", 
"WM_AWT_OPENCANDIDATEWINDOW" I do not find calls to "Imm*" system 
functions. Therefore maybe there is no need to send these messages 
through "PostMessage"?

Thank you,
Anton

On 18/08/2020 06:46, Dmitry Markov wrote:
> Thank you, Sergey!
>
> Looking for one more "+1”. Any volunteers?
>
> Regards,
> Dmitry
>> On 17 Aug 2020, at 21:06, Sergey Bylokhov <sergey.bylokhov at oracle.com 
>> <mailto:sergey.bylokhov at oracle.com>> wrote:
>>
>> Looks fine.
>>
>> On 17.08.2020 02:32, Dmitry Markov wrote:
>>> Hi Sergey,
>>> I have added that information to InvokeInputMethodFunction(). Please 
>>> find the new webrev 
>>> here:http://cr.openjdk.java.net/~dmarkov/8232114/webrev.01/
>>> Regards,
>>> Dmitry
>>>> On 15 Aug 2020, at 03:05, Sergey Bylokhov 
>>>> <sergey.bylokhov at oracle.com 
>>>> <mailto:sergey.bylokhov at oracle.com><mailto:sergey.bylokhov at oracle.com>> 
>>>> wrote:
>>>>
>>>> On 12.08.2020 05:09, Dmitry Markov wrote:
>>>>> TranslateMessage() does not invoke PeekMessage(). In our case 
>>>>> TranslateMessage() is called by AWT. IME functionality may call 
>>>>> PeekMessage() during TranslateMessage() execution. However that 
>>>>> PeekMessage() call is intended for processing non-queued 
>>>>>  messages, (i.e. the messages send via SendMessage() call).
>>>>> I contacted Microsoft regarding this problem and one of their 
>>>>> suggestions was to use PostMessage() instead of SendMessage() for 
>>>>> IME messages to avoid IME internal data corruption and the crash.
>>>>> The proposed fix was tested by the stress test for several weeks 
>>>>> and no issues were observed. So I feel quite confident that it 
>>>>> eliminates the issue.
>>>>> There is no exact message which triggers the crash. Usually the 
>>>>> crash is caused by one of the following messages: 
>>>>> WM_AWT_ASSOCIATECONTEXT or WM_AWT_SETOPENSTATUS but several times 
>>>>> I observed that it was triggered by WM_AWT_DESTROYCONTEXT or 
>>>>> WM_AWT_CREATECONTEXT. It looks like almost every IME-related 
>>>>> message may cause the crash.  I think SendMessage() call should be 
>>>>> substituted by PostMessage() for all IME messages.
>>>>
>>>> Ok, then please add this(or similar) information to the new method 
>>>> "InvokeInputMethodFunction",
>>>> otherwise it could be removed in the future/replaced back to the 
>>>> sendMessage.
>>>>
>>>>> Regards,
>>>>> Dmitry
>>>>>> On 12 Aug 2020, at 06:16, Sergey Bylokhov 
>>>>>> <Sergey.Bylokhov at oracle.com 
>>>>>> <mailto:Sergey.Bylokhov at oracle.com><mailto:Sergey.Bylokhov at oracle.com>> 
>>>>>> wrote:
>>>>>>
>>>>>> Hi, Dmitry.
>>>>>>
>>>>>> On 11.08.2020 01:07, Dmitry Markov wrote:
>>>>>>> Problem description:
>>>>>>> The root cause of the crash is the lack of synchronisation in 
>>>>>>> imjpapi.dll. In particular when IME messages are processed in 
>>>>>>> the message loop and another message triggered through a 
>>>>>>> SendMessage() call, this clears the buffer context so on further 
>>>>>>> processing the message loop in IME context will point to invalid 
>>>>>>> memory buffer. Microsoft article devoted to this issue: 
>>>>>>> https://docs.microsoft.com/en-us/troubleshoot/windows/win32/ime-crash-processing-cross-thread-sent-message
>>>>>>
>>>>>> The documentation above also states that PeekMessage, called by 
>>>>>> the TranslateMessage when the IME is ON, can proceed the posted 
>>>>>> messages as well if that true then the current fix does not help.
>>>>>>
>>>>>>> Fix:
>>>>>>> Replace SendMessage() with PostMessage() for IME messages and 
>>>>>>> implement event based mechanism to notify the sender that the 
>>>>>>> message processing is completed.
>>>>>>
>>>>>> What exact message broke the IME, the "WM_AWT_DESTROYCONTEXT"?
>>>>>>
>>>>>>> Testing:
>>>>>>> mach5 client tests (jtreg headful, jck, etc.) are green.
>>>>>>> Regards,
>>>>>>> Dmitry
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Best regards, Sergey.
>>>>
>>>>
>>>> --
>>>> Best regards, Sergey.
>>
>>
>> --
>> Best regards, Sergey.
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/awt-dev/attachments/20200820/4143986e/attachment.htm>